Improve handling of Vasprun POTCAR search, expanded fake POTCAR library for VASP I/O tests (#3491)

* Added fake potcar library for tests

* fix failing lobster test caused by removal of potcars

* fix failing io.vasp.test_sets

* fix alchemy.test_materials test

* Add pymatgen.util.testing.FAKE_POTCAR_DIR var for fake potcar library, rename directory of fake potcars

* refactor POTCAR search path in Vasprun.get_potcars

* test_inputs.py move monkey-patching of potcar_summary_stats into new _mock_complete_potcar_summary_stats fixture

* remove largely duplicate asserts assert input_set.potcar.functional == "PBE_64" in TestMatPESStaticSet

* minimize diff

* restore comments

* Modify POTCAR scrambling to only modify parts of the POTCAR not printed to OUTCAR. Revise test data and fake POTCAR library

* Make potcar_summary_stats a private attr of pymatgen.io.vasp.inputs.PotcarSingle to resolve slow docs loading time

* update doc strings, fix typo

* add comment explaining FAKE_POTCAR_DIR contents

---------

Signed-off-by: Aaron Kaplan <[email protected]>
Co-authored-by: Janosh Riebesell <[email protected]>
Co-authored-by: Aaron Kaplan <[email protected]>

Author: 3 people

dev_scripts/potcar_scrambler.py

@@ -3,6 +3,7 @@
 import os
 import shutil
 import warnings
+from glob import glob
 
 import numpy as np
 from monty.os.path import zpath
@@ -68,28 +69,59 @@ def _read_fortran_str_and_scramble(self, input_str: str, bloat: float = 1.5):
             return input_str
 
     def scramble_single_potcar(self, potcar: PotcarSingle):
+        """
+        Scramble the body of a POTCAR, retain the PSCTR header information.
+
+        To the best of my (ADK) knowledge, in the OUTCAR file,
+        almost all information from the POTCAR in the "PSCTR" block
+            ```
+            parameters from PSCTR are:
+            ....
+            END of PSCTR-controll parameters
+            ```
+        is printed to OUTCAR. Specifically, all information above the line
+            `Error from kinetic energy argument (eV)`
+        is included. This information is not scrambled below.
+        """
         scrambled_potcar_str = ""
+        needs_sha256 = False
+        scramble_values = False
+        og_sha_str = "SHA256 = None\n"
         for line in potcar.data.split("\n")[:-1]:
             single_line_rows = line.split(";")
-            if "SHA256" in line or "COPYR" in line:
-                # files not copyrighted, remove copyright statement
-                # sha256 no longer applicable
+
+            if "SHA256" in line:
+                scrambled_potcar_str += og_sha_str
+                needs_sha256 = True
                 continue
 
+            if ("Error from kinetic energy argument (eV)" in line) or ("END of PSCTR-controll parameters" in line):
+                # start to scramble values, logic described above
+                scramble_values = True
+
             cline = ""
             for idx, row in enumerate(single_line_rows):
-                split_row = row.split()
-                for itmp, tmp in enumerate(split_row):
-                    cline += f"{self._read_fortran_str_and_scramble(tmp)}"
-                    if itmp < len(split_row) - 1:
-                        cline += " "
+                if scramble_values:
+                    split_row = row.split()
+                    for itmp, tmp in enumerate(split_row):
+                        cline += f"{self._read_fortran_str_and_scramble(tmp)}"
+                        if itmp < len(split_row) - 1:
+                            cline += " "
+                else:
+                    cline += row
                 if len(single_line_rows) > 1 and idx == 0:
                     cline += "; "
 
             aux_str = ""
             if "TITEL" in line:
                 aux_str = " FAKE"
             scrambled_potcar_str += f"{cline}{aux_str}\n"
+
+        if needs_sha256:
+            tps = PotcarSingle(scrambled_potcar_str)
+            scrambled_potcar_str = scrambled_potcar_str.replace(
+                og_sha_str, f"SHA256 = {tps.sha256_computed_file_hash}\n"
+            )
         return scrambled_potcar_str
 
     def to_file(self, filename: str):
@@ -141,5 +173,28 @@ def generate_fake_potcar_libraries():
                     break
 
 
+def potcar_cleanser():
+    """
+    Function to replace copyrighted POTCARs used in io.vasp.sets testing
+    with dummy POTCARs that have scrambled PSP and kinetic energy values
+    (but retain the original header information which is also found in OUTCARs
+    and freely shared by VASP)
+    """
+
+    search_dir = "../tests/files/fake_potcars/real_potcars/"
+    rebase_dir = search_dir.replace("real", "fake")
+    potcars_to_cleanse = glob(f"{search_dir}/**/POTCAR*", recursive=True)
+
+    for potcar in potcars_to_cleanse:
+        path_to_potcar, potcar_name = potcar.split("POTCAR")
+        rebased = path_to_potcar.replace(search_dir, rebase_dir)
+        new_path = f"{rebased}POTCAR{potcar_name}"
+        if new_path[-3:] != ".gz":
+            new_path += ".gz"
+        os.makedirs(rebased, exist_ok=True)
+        PotcarScrambler.from_file(input_filename=potcar, output_filename=new_path)
+
+
 if __name__ == "__main__":
-    generate_fake_potcar_libraries()
+    potcar_cleanser()
+    # generate_fake_potcar_libraries()

pymatgen/io/vasp/inputs.py

@@ -1691,7 +1691,7 @@ class PotcarSingle:
     )
 
     # used for POTCAR validation
-    potcar_summary_stats = loadfn(POTCAR_STATS_PATH)
+    _potcar_summary_stats = loadfn(POTCAR_STATS_PATH)
 
     def __init__(self, data: str, symbol: str | None = None) -> None:
         """
@@ -1975,7 +1975,7 @@ def identify_potcar(
 
         identity: dict[str, list] = {"potcar_functionals": [], "potcar_symbols": []}
         for func in self.functional_dir:
-            for ref_psp in self.potcar_summary_stats[func].get(self.TITEL.replace(" ", ""), []):
+            for ref_psp in self._potcar_summary_stats[func].get(self.TITEL.replace(" ", ""), []):
                 if self.VRHFIN.replace(" ", "") != ref_psp["VRHFIN"]:
                     continue
 
@@ -2243,9 +2243,9 @@ def is_valid(self) -> bool:
         consistent values of LEXCH
         """
         for func in self.functional_dir:
-            for titel_no_spc in self.potcar_summary_stats[func]:
+            for titel_no_spc in self._potcar_summary_stats[func]:
                 if self.TITEL.replace(" ", "") == titel_no_spc:
-                    for potcar_subvariant in self.potcar_summary_stats[func][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(
                                 {
@@ -2370,7 +2370,7 @@ def __repr__(self) -> str:
 
 
 def _gen_potcar_summary_stats(
-    append: bool = False, vasp_psp_dir: str | None = None, summary_stats_filename: str = POTCAR_STATS_PATH
+    append: bool = False, vasp_psp_dir: str | None = None, summary_stats_filename: str | None = POTCAR_STATS_PATH
 ):
     """
     This function solely intended to be used for PMG development to regenerate the
@@ -2426,7 +2426,10 @@ def _gen_potcar_summary_stats(
                 }
             )
 
-    dumpfn(new_summary_stats, summary_stats_filename)
+    if summary_stats_filename:
+        dumpfn(new_summary_stats, summary_stats_filename)
+
+    return new_summary_stats
 
 
 class Potcar(list, MSONable):

pymatgen/io/vasp/outputs.py

@@ -993,35 +993,34 @@ def get_potcars(self, path: str | Path) -> Potcar | None:
         Returns the POTCAR from the specified path.
 
         Args:
-            path (str): The path to search for POTCARs.
+            path (str | Path): The path to search for POTCARs.
 
         Returns:
-            Potcar | None: The POTCAR from the specified path.
+            Potcar | None: The POTCAR from the specified path or None if not found/no path specified.
         """
 
-        def get_potcar_in_path(p):
-            for fn in os.listdir(os.path.abspath(p)):
-                if fn.startswith("POTCAR") and ".spec" not in fn:
-                    pc = Potcar.from_file(os.path.join(p, fn))
-                    if {d.header for d in pc} == set(self.potcar_symbols):
-                        return pc
-            warnings.warn(f"No POTCAR file with matching TITEL fields was found in {os.path.abspath(p)}")
+        if not path:
             return None
 
-        if isinstance(path, (str, Path)):
-            path = str(path)
-            if "POTCAR" in path:
-                potcar = Potcar.from_file(path)
-                if {d.TITEL for d in potcar} != set(self.potcar_symbols):
-                    raise ValueError("Potcar TITELs do not match Vasprun")
-            else:
-                potcar = get_potcar_in_path(path)
-        elif isinstance(path, bool) and path:
-            potcar = get_potcar_in_path(os.path.split(self.filename)[0])
+        if isinstance(path, (str, Path)) and "POTCAR" in str(path):
+            potcar_paths = [str(path)]
         else:
-            potcar = None
+            search_path = os.path.split(self.filename)[0] if path is True else str(path)
+            potcar_paths = [
+                f"{search_path}/{fn}" for fn in os.listdir(search_path) if fn.startswith("POTCAR") and ".spec" not in fn
+            ]
 
-        return potcar
+        for potcar_path in potcar_paths:
+            try:
+                potcar = Potcar.from_file(potcar_path)
+                if {d.header for d in potcar} == set(self.potcar_symbols):
+                    return potcar
+            except Exception:
+                continue
+
+        warnings.warn("No POTCAR file with matching TITEL fields was found in\n" + "\n  ".join(potcar_paths))
+
+        return None
 
     def get_trajectory(self):
         """

pymatgen/io/vasp/sets.py

@@ -1270,7 +1270,9 @@ def __init__(
             self._config_dict["INCAR"].pop("GGA", None)
         if xc_functional.upper().endswith("+U"):
             self._config_dict["INCAR"]["LDAU"] = True
-        default_potcars = self.CONFIG["PARENT"].replace("PBE", "PBE_").replace("BASE", "")  # PBE64BASE -> PBE_64
+
+        default_potcars = self.CONFIG["PARENT"].upper()
+        default_potcars = default_potcars.replace("PBE", "PBE_").replace("BASE", "")  # PBE64BASE -> PBE_64
         user_potcar_functional = kwargs.get("user_potcar_functional", default_potcars)
         if user_potcar_functional.upper() != default_potcars:
             warnings.warn(

pymatgen/util/testing.py

@@ -26,6 +26,10 @@
 MODULE_DIR = Path(__file__).absolute().parent
 STRUCTURES_DIR = MODULE_DIR / "structures"
 TEST_FILES_DIR = Path(SETTINGS.get("PMG_TEST_FILES_DIR", MODULE_DIR / ".." / ".." / "tests" / "files"))
+# fake POTCARs have original header information, meaning properties like number of electrons,
+# nuclear charge, core radii, etc. are unchanged (important for testing) while values of the and
+# pseudopotential kinetic energy corrections are scrambled to avoid VASP copyright infringement
+FAKE_POTCAR_DIR = TEST_FILES_DIR / "fake_potcars"
 
 
 class PymatgenTest(unittest.TestCase):

tests/alchemy/test_materials.py

@@ -15,7 +15,7 @@
     SupercellTransformation,
 )
 from pymatgen.util.provenance import StructureNL
-from pymatgen.util.testing import TEST_FILES_DIR, PymatgenTest
+from pymatgen.util.testing import FAKE_POTCAR_DIR, TEST_FILES_DIR, PymatgenTest
 
 
 class TestTransformedStructure(PymatgenTest):
@@ -49,7 +49,7 @@ def test_append_filter(self):
         self.trans.append_filter(f3)
 
     def test_get_vasp_input(self):
-        SETTINGS["PMG_VASP_PSP_DIR"] = TEST_FILES_DIR
+        SETTINGS["PMG_VASP_PSP_DIR"] = FAKE_POTCAR_DIR
         potcar = self.trans.get_vasp_input(MPRelaxSet)["POTCAR"]
         assert "\n".join(p.symbol for p in potcar) == "Na_pv\nFe_pv\nP\nO"
         assert len(self.trans.structures) == 2