Add new .pmgrc.yaml setting PMG_VASP_PSP_SUB_DIRS: dict[str, str] (#3858)

* add new PMG_VASP_PSP_SUB_DIRS SETTING used in from_symbol_and_functional to allow customizing currently hard-coded subfolder structure of PMG_VASP_PSP_DIR

* fix TestMatPESStaticSet.test_default and TestLobsterSet.test_potcar

* add TestPotcarSingle.test_from_symbol_and_functional_raises

* uncomment and fix test_default_functional

* del outdated test.yml comment

thanks @DanielYang59

* try fix test_from_symbol_and_functional_raises on windows

Author: janosh

.github/workflows/test.yml

@@ -28,8 +28,7 @@ jobs:
         # maximize CI coverage of different platforms and python versions while minimizing the
         # total number of jobs. We run all pytest splits with the oldest supported python
         # version (currently 3.9) on windows (seems most likely to surface errors) and with
-        # newest version (currently 3.12) on ubuntu (to get complete coverage on unix). We
-        # ignore mac-os, which is assumed to be similar to ubuntu.
+        # newest version (currently 3.12) on ubuntu (to get complete coverage on unix).
         config:
           - os: windows-latest
             python: "3.9"
@@ -39,11 +38,10 @@ jobs:
             python: "3.12"
             resolution: lowest-direct
             extras: ci,optional
-          # test with lowest resolution and only required dependencies installed
           - os: macos-latest
             python: "3.10"
             resolution: lowest-direct
-            extras: ci
+            extras: ci # test with only required dependencies installed
 
         # pytest-split automatically distributes work load so parallel jobs finish in similar time
         split: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]

pymatgen/io/vasp/inputs.py

@@ -2126,13 +2126,8 @@ def is_valid(self) -> bool:
                 if self.TITEL.replace(" ", "") == titel_no_spc:
                     for potcar_subvariant in self._potcar_summary_stats[func][titel_no_spc]:
                         if self.VRHFIN.replace(" ", "") == potcar_subvariant["VRHFIN"]:
-                            possible_potcar_matches.append(
-                                {
-                                    "POTCAR_FUNCTIONAL": func,
-                                    "TITEL": titel_no_spc,
-                                    **potcar_subvariant,
-                                }
-                            )
+                            possible_match = {"POTCAR_FUNCTIONAL": func, "TITEL": titel_no_spc, **potcar_subvariant}
+                            possible_potcar_matches.append(possible_match)
 
         def parse_fortran_style_str(input_str: str) -> str | bool | float | int:
             """Parse any input string as bool, int, float, or failing that, str.
@@ -2284,26 +2279,29 @@ def from_symbol_and_functional(
         if functional is None:
             raise ValueError("Cannot get functional.")
 
-        funcdir = cls.functional_dir[functional]
+        functional_subdir = SETTINGS.get("PMG_VASP_PSP_SUB_DIRS", {}).get(functional, cls.functional_dir[functional])
         PMG_VASP_PSP_DIR = SETTINGS.get("PMG_VASP_PSP_DIR")
         if PMG_VASP_PSP_DIR is None:
             raise ValueError(
                 f"No POTCAR for {symbol} with {functional=} found. Please set the PMG_VASP_PSP_DIR in .pmgrc.yaml."
             )
+        if not os.path.isdir(PMG_VASP_PSP_DIR):
+            raise FileNotFoundError(f"{PMG_VASP_PSP_DIR=} does not exist.")
 
         paths_to_try: list[str] = [
-            os.path.join(PMG_VASP_PSP_DIR, funcdir, f"POTCAR.{symbol}"),
-            os.path.join(PMG_VASP_PSP_DIR, funcdir, symbol, "POTCAR"),
+            os.path.join(PMG_VASP_PSP_DIR, functional_subdir, f"POTCAR.{symbol}"),
+            os.path.join(PMG_VASP_PSP_DIR, functional_subdir, symbol, "POTCAR"),
         ]
+        path = paths_to_try[0]
         for path in paths_to_try:
             path = os.path.expanduser(path)
             path = zpath(path)
             if os.path.isfile(path):
                 return cls.from_file(path)
 
-        raise RuntimeError(
-            f"You do not have the right POTCAR with {functional=} and {symbol=} "
-            f"in your {PMG_VASP_PSP_DIR=}. Paths tried: {paths_to_try}"
+        raise FileNotFoundError(
+            f"You do not have the right POTCAR with {functional=} and {symbol=}\n"
+            f"in your {PMG_VASP_PSP_DIR=}.\nPaths tried:\n- " + "\n- ".join(paths_to_try)
         )
 
     def verify_potcar(self) -> tuple[bool, bool]:

tests/io/vasp/test_inputs.py

@@ -7,6 +7,7 @@
 import re
 from shutil import copyfile
 from unittest import TestCase
+from unittest.mock import patch
 
 import numpy as np
 import pytest
@@ -1182,20 +1183,43 @@ def test_multi_potcar_with_and_without_sha256(self):
             else:
                 assert psingle.is_valid
 
-    # def test_default_functional(self):
-    #     potcar = PotcarSingle.from_symbol_and_functional("Fe")
-    #     assert potcar.functional_class == "GGA"
-    #     SETTINGS["PMG_DEFAULT_FUNCTIONAL"] = "LDA"
-    #     potcar = PotcarSingle.from_symbol_and_functional("Fe")
-    #     assert potcar.functional_class == "LDA"
-    #     SETTINGS["PMG_DEFAULT_FUNCTIONAL"] = "PBE"
+    def test_default_functional(self):
+        with patch.dict(SETTINGS, PMG_DEFAULT_FUNCTIONAL="PBE"):
+            potcar = PotcarSingle.from_symbol_and_functional("Fe")
+            assert potcar.functional_class == "GGA"
+        with patch.dict(SETTINGS, PMG_DEFAULT_FUNCTIONAL="LDA"):
+            SETTINGS["PMG_DEFAULT_FUNCTIONAL"] = "LDA"
+            potcar = PotcarSingle.from_symbol_and_functional("Fe")
+            assert potcar.functional_class == "LDA"
+
+    def test_from_symbol_and_functional_raises(self):
+        # test FileNotFoundError on non-existent PMG_VASP_PSP_DIR in SETTINGS
+        PMG_VASP_PSP_DIR = "missing-dir"
+        symbol, functional = "Fe", "PBE_64"
+        with (
+            patch.dict(SETTINGS, PMG_VASP_PSP_DIR=PMG_VASP_PSP_DIR),
+            pytest.raises(FileNotFoundError, match=f"{PMG_VASP_PSP_DIR=} does not exist."),
+        ):
+            PotcarSingle.from_symbol_and_functional(symbol, functional)
+
+        # test different FileNotFoundError on non-existent POTCAR sub-directory
+        PMG_VASP_PSP_DIR = SETTINGS["PMG_VASP_PSP_DIR"]
+        err_msg = f"You do not have the right POTCAR with {functional=} and {symbol=}\nin your {PMG_VASP_PSP_DIR=}"
+
+        with (
+            patch.dict(SETTINGS, PMG_VASP_PSP_SUB_DIRS={"PBE_64": "PBE_64_FOO"}),
+            pytest.raises(FileNotFoundError) as exc_info,
+        ):
+            PotcarSingle.from_symbol_and_functional(symbol, functional)
+
+        assert err_msg in str(exc_info.value)
 
     def test_repr(self):
-        assert (
-            repr(self.psingle_Mn_pv)
-            == "PotcarSingle(symbol='Mn_pv', functional='PBE', TITEL='PAW_PBE Mn_pv 07Sep2000', "
+        expected_repr = (
+            "PotcarSingle(symbol='Mn_pv', functional='PBE', TITEL='PAW_PBE Mn_pv 07Sep2000', "
             "VRHFIN='Mn: 3p4s3d', n_valence_elec=13)"
         )
+        assert repr(self.psingle_Mn_pv) == expected_repr
 
     def test_hash(self):
         assert self.psingle_Mn_pv.md5_header_hash == "b45747d8ceeee91c3b27e8484db32f5a"

tests/io/vasp/test_sets.py

@@ -851,8 +851,8 @@ def test_default(self):
 
         assert input_set.potcar_functional == "PBE_64"  # test POTCARs default to PBE_64
         assert input_set.kpoints is None
-        # only runs if POTCAR files to compare against are available
-        if os.path.isdir(f"{FAKE_POTCAR_DIR}/POT_GGA_PAW_PBE_64"):
+        if os.path.isdir(f"{FAKE_POTCAR_DIR}/POT_PAW_PBE_64"):
+            # this part only runs if POTCAR files are available
             assert str(input_set.potcar[0]) == str(PotcarSingle.from_symbol_and_functional("Fe_pv", "PBE_64"))
 
     def test_with_prev_incar(self):
@@ -1980,12 +1980,13 @@ def test_kpoints(self):
     @skip_if_no_psp_dir
     def test_potcar(self):
         # PBE_54 is preferred at the moment
-        assert self.lobsterset1.user_potcar_functional == "PBE_54"
+        functional, symbol = "PBE_54", "K_sv"
+        assert self.lobsterset1.user_potcar_functional == functional
         # test if potcars selected are consistent with PBE_54
         assert self.lobsterset2.potcar.symbols == ["Fe_pv", "P", "O"]
         # test if error raised contains correct potcar symbol for K element as PBE_54 set
         with pytest.raises(
-            RuntimeError, match="You do not have the right POTCAR with functional='PBE_54' and symbol='K_sv'"
+            FileNotFoundError, match=f"You do not have the right POTCAR with {functional=} and {symbol=}"
         ):
             _ = self.lobsterset9.potcar.symbols