Merge branch 'master' of github.com:materialsproject/pymatgen

Author: shyuep

src/pymatgen/io/jdftx/generic_tags.py

@@ -430,6 +430,7 @@ class FloatTag(AbstractTag):
     """
 
     prec: int | None = None
+    minval: float | None = None
 
     def validate_value_type(self, tag: str, value: Any, try_auto_type_fix: bool = False) -> tuple[str, bool, Any]:
         """Validate the type of the value for this tag.
@@ -472,6 +473,11 @@ def write(self, tag: str, value: Any) -> str:
         Returns:
             str: The tag and its value as a string.
         """
+        # Returning an empty string instead of raising an error as value == self.minval
+        # will cause JDFTx to throw an error, but the internal infile dumps the value as
+        # as the minval if not set by the user.
+        if (self.minval is not None) and (not value > self.minval):
+            return ""
         # pre-convert to string: self.prec+3 is minimum room for:
         # - sign, 1 integer left of decimal, decimal, and precision.
         # larger numbers auto add places to left of decimal

src/pymatgen/io/jdftx/inputs.py

@@ -46,6 +46,10 @@ class is written.
 
 # TODO: Add check for whether all ions have or lack velocities.
 # TODO: Add default value filling like JDFTx does.
+# TODO: Add more robust checking for if two repeatable tag values represent the
+# same information. This is likely fixed by implementing filling of default values.
+# TODO: Incorporate something to collapse repeated dump tags of the same frequency
+# into a single value.
 
 
 class JDFTXInfile(dict, MSONable):
@@ -80,18 +84,34 @@ def __add__(self, other: JDFTXInfile) -> JDFTXInfile:
         """Add existing JDFTXInfile object to method caller JDFTXInfile object.
 
         Add all the values of another JDFTXInfile object to this object. Facilitate the use of "standard" JDFTXInfiles.
+        Repeatable tags are appended together. Non-repeatable tags are replaced with their value from the other object.
 
         Args:
             other (JDFTXInfile): JDFTXInfile object to add to the method caller object.
 
         Returns:
             JDFTXInfile: The combined JDFTXInfile object.
         """
-        params: dict[str, Any] = dict(self.items())
+        # Deepcopy needed here, or else in `jif1 = jif2 + jif3`, `jif1` will become a reference to `jif2`
+        params: dict[str, Any] = deepcopy(dict(self.items()))
         for key, val in other.items():
-            if key in self and val != self[key]:
-                raise ValueError(f"JDFTXInfiles have conflicting values for {key}: {self[key]} != {val}")
-            params[key] = val
+            if key in self:
+                if val is params[key]:
+                    # Unlinking the two objects fully cannot be done by deepcopy for some reason
+                    continue
+                tag_object = get_tag_object(key)
+                if tag_object.can_repeat:
+                    if isinstance(val, list):
+                        for subval in list(val):
+                            # Minimum effort to avoid duplicates
+                            if subval not in params[key]:
+                                params[key].append(subval)
+                    else:
+                        params[key].append(val)
+                else:
+                    params[key] = val
+            else:
+                params[key] = val
         return type(self)(params)
 
     def as_dict(self, sort_tags: bool = True, skip_module_keys: bool = False) -> dict:
@@ -553,6 +573,17 @@ def validate_tags(
                     warnmsg += "(Check earlier warnings for more details)\n"
                 warnings.warn(warnmsg, stacklevel=2)
 
+    def strip_structure_tags(self) -> None:
+        """Strip all structural tags from the JDFTXInfile.
+
+        Strip all structural tags from the JDFTXInfile. This is useful for preparing a JDFTXInfile object
+        from a pre-existing calculation for a new structure.
+        """
+        strucural_tags = ["lattice", "ion", "lattice-scale", "coords-type"]
+        for tag in strucural_tags:
+            if tag in self:
+                del self[tag]
+
     def __setitem__(self, key: str, value: Any) -> None:
         """Set an item in the JDFTXInfile.
 

src/pymatgen/io/jdftx/jdftxinfile_ref_options.py

@@ -792,15 +792,15 @@
     "knormThreshold": FloatTag(),
     "linminMethod": StrTag(options=["CubicWolfe", "DirUpdateRecommended", "Quad", "Relax"]),
     "maxThreshold": BoolTag(),
-    "nAlphaAdjustMax": FloatTag(),
+    "nAlphaAdjustMax": IntTag(),
     "nEnergyDiff": IntTag(),
     "nIterations": IntTag(),
     "updateTestStepSize": BoolTag(),
     "wolfeEnergy": FloatTag(),
     "wolfeGradient": FloatTag(),
 }
 jdftxfluid_subtagdict = {
-    "epsBulk": FloatTag(),
+    "epsBulk": FloatTag(minval=1.0),
     "epsInf": FloatTag(),
     "epsLJ": FloatTag(),
     "Nnorm": FloatTag(),
@@ -814,12 +814,12 @@
             "A0": FloatTag(write_tagname=False, optional=False),
         },
     ),
-    "Pvap": FloatTag(),
+    "Pvap": FloatTag(minval=0.0),
     "quad_nAlpha": FloatTag(),
     "quad_nBeta": FloatTag(),
     "quad_nGamma": FloatTag(),
     "representation": TagContainer(subtags={"MuEps": FloatTag(), "Pomega": FloatTag(), "PsiAlpha": FloatTag()}),
-    "Res": FloatTag(),
+    "Res": FloatTag(minval=0.0),
     "Rvdw": FloatTag(),
     "s2quadType": StrTag(
         options=[
@@ -844,7 +844,7 @@
             "Tetrahedron",
         ]
     ),
-    "sigmaBulk": FloatTag(),
+    "sigmaBulk": FloatTag(minval=0.0),
     "tauNuc": FloatTag(),
     "translation": StrTag(options=["ConstantSpline", "Fourier", "LinearSpline"]),
 }

src/pymatgen/io/jdftx/jdftxoutfileslice.py

@@ -391,7 +391,10 @@ def _set_internal_infile(self, text: list[str]) -> None:
         Args:
             text (list[str]): Output of read_file for out file.
         """
-        start_line_idx = find_key("Input parsed successfully", text) + 2
+        start_line_idx = find_key("Input parsed successfully", text)
+        if start_line_idx is None:
+            raise ValueError("JDFTx input parsing failed on most recent call.")
+        start_line_idx += 2
         end_line_idx = None
         for i in range(start_line_idx, len(text)):
             if not len(text[i].strip()):
@@ -1157,6 +1160,24 @@ def determine_is_metal(self) -> bool | None:
             raise ValueError("Cannot determine if system is metal - self.nspin undefined")
         return None
 
+    def to_jdftxinfile(self) -> JDFTXInfile:
+        """
+        Convert the JDFTXOutfile object to a JDFTXInfile object with the most recent structure.
+        If the input structure is desired, simply fetch JDFTXOutfile.infile
+
+        Returns:
+            JDFTXInfile: A JDFTXInfile object representing the input parameters of the JDFTXOutfile.
+        """
+        # Use internal infile as a reference for calculation parameters
+        base_infile = self.infile.copy()
+        # Strip references to the input
+        base_infile.strip_structure_tags()
+        if self.structure is None:
+            return base_infile
+        infile = JDFTXInfile.from_structure(self.structure)
+        infile += base_infile
+        return infile
+
     def _check_solvation(self) -> bool:
         """Check for implicit solvation.
 

src/pymatgen/io/jdftx/outputs.py

@@ -27,6 +27,7 @@ class is written.
 if TYPE_CHECKING:
     from pymatgen.core.structure import Structure
     from pymatgen.core.trajectory import Trajectory
+    from pymatgen.io.jdftx.inputs import JDFTXInfile
     from pymatgen.io.jdftx.jelstep import JElSteps
     from pymatgen.io.jdftx.jminsettings import (
         JMinSettingsElectronic,
@@ -287,8 +288,43 @@ def _store_eigenvals(self):
     "electronic_output",
     "t_s",
     "ecomponents",
+    "infile",
 )
 
+# TODO: Remove references to the deprecated 'jsettings_*' attributes in `JDFTXOutfile` and `JDFTXOutfileSlice`
+# TODO: JDFTXOutfile and JDFTXOutfileSlice are VERY bloated. The following attributes are redundant
+# to fields of the `Structure` object and their removal likely won't cause any confusion.
+# - lattice
+# - lattice_final
+# - atom_coords
+# - atom_coords_final
+# - a/b/c
+# The following attributes are redundant to the internal `JDFTXInfile` object, but since the JDFTXInfile
+# object is not a standard pymatgen object, it may be better to decorate them with a deprecated decorator
+# until end of year. Additionally, it should be made certain which of these are dumped in the outfile's
+# input section regardless of what was set in the input file - for those that are not, they need to be kept.
+# - lattice_initial
+# - atom_coords_initial
+# - broadening
+# - broadening_type
+# - kgrid
+# - truncation_type
+# - truncation_radius
+# - fluid
+# - pwcut
+# - rhocut
+# The following are attributes that come from the electronic/fluid/ionic/lattice optimization step
+# logs. I am not sure if it is actually helpful to have access to these from the `JDFTXOutfile` object,
+# as they are only informative for analyzing optimization convergence. Additionally, the fields are
+# less universal than I thought, and can change depending on the optimization settings, so it might
+# be smarter to store them in a dictionary of arbitrary keys instead within the contained substructures.
+# - grad_k
+# - alpha
+# - linmin
+# - elec_grad_k
+# - elec_alpha
+# - elec_linmin
+
 
 @dataclass
 class JDFTXOutfile:
@@ -420,6 +456,7 @@ class JDFTXOutfile:
         elec_alpha (float): The step size of the final electronic step in the most recent JDFTx call.
         elec_linmin (float): The final normalized projection of the electronic step direction onto the gradient for the
             most recent JDFTx call.
+        infile (JDFTXInfile): The JDFTXInfile object representing the input parameters of the JDFTXOutfile.
 
     Magic Methods:
         __getitem__(key: str | int) -> Any: Decides behavior of how JDFTXOutfile objects are indexed. If the key is a
@@ -506,6 +543,7 @@ class JDFTXOutfile:
     elec_alpha: float = field(init=False)
     elec_linmin: float = field(init=False)
     electronic_output: float = field(init=False)
+    infile: JDFTXInfile = field(init=False)
 
     @classmethod
     def from_calc_dir(
@@ -558,6 +596,17 @@ def from_file(
         ]
         return cls(slices=slices)
 
+    # TODO: Write testing for this function
+    def to_jdftxinfile(self) -> JDFTXInfile:
+        """
+        Convert the JDFTXOutfile object to a JDFTXInfile object with the most recent structure.
+        If the input structure is desired, simply fetch JDFTXOutfile.infile
+
+        Returns:
+            JDFTXInfile: A JDFTXInfile object representing the input parameters of the JDFTXOutfile.
+        """
+        return self.slices[-1].to_jdftxinfile()
+
     def __post_init__(self):
         last_slice = None
         for slc in self.slices[::-1]:

tests/io/jdftx/test_jdftxinfile.py

@@ -186,6 +186,14 @@ def test_JDFTXInfile_expected_exceptions():
         jif[1.2] = 3.4
 
 
+def test_JDFTXInfile_strip_structure():
+    jif = JDFTXInfile.from_file(ex_infile1_fname)
+    structural_tags = ["lattice", "ion", "coords-type"]
+    assert all(tag in jif for tag in structural_tags)
+    jif.strip_structure_tags()
+    assert all(tag not in jif for tag in structural_tags)
+
+
 def test_JDFTXInfile_niche_cases():
     jif = JDFTXInfile.from_file(ex_infile1_fname)
     tag_object, tag, value = jif._preprocess_line("dump-only")
@@ -214,6 +222,7 @@ def test_JDFTXInfile_add_method():
     # repeatable values, repeatable tags are not append to each other
     jif = JDFTXInfile.from_file(ex_infile1_fname)
     jif2 = jif.copy()
+    assert jif2 is not jif  # Testing robustness of copy method while we are at it
     jif3 = jif + jif2
     assert_idential_jif(jif, jif3)
     # If a tag is repeated, the values must be the same since choice of value is ambiguous
@@ -222,9 +231,17 @@ def test_JDFTXInfile_add_method():
     val_new = "lda"
     assert val_old != val_new
     jif2[key] = val_new
-    err_str = f"JDFTXInfiles have conflicting values for {key}: {val_old} != {val_new}"
-    with pytest.raises(ValueError, match=re.escape(err_str)):
-        jif3 = jif + jif2
+    jif4 = jif + jif2
+    assert_same_value(jif4[key], val_new)  # Make sure addition chooses second value for non-repeatable tags
+    del jif4
+    jif2.append_tag("dump", "Fluid State")
+    jif.append_tag("dump", "Fluid Berry")
+    jif4 = jif + jif2
+    assert len(jif4["dump"]) == len(jif2["dump"]) + 1
+    assert {"Fluid": {"State": True}} in jif4["dump"]
+    assert {"Fluid": {"State": True}} not in jif["dump"]
+    assert {"Fluid": {"Berry": True}} in jif4["dump"]
+    assert {"Fluid": {"Berry": True}} not in jif2["dump"]
     # Normal expected behavior
     key_add = "target-mu"
     val_add = 0.5