JDFTXOutfile none_on_error oversight fix (#4399)

* Fixing oversight where attributes are incorrect retrieved if the final outfile slice is `None`, removing commented out code

* Testing for nonetype fix, expanding generic assert_same_value function

Author: benrich37

src/pymatgen/io/jdftx/jdftxoutfileslice.py

@@ -826,40 +826,6 @@ def _create_settings_dict(self, text: list[str], start_flag: str) -> dict:
             settings_dict[key] = value
         return settings_dict
 
-    # def _get_settings_object(
-    #     self,
-    #     text: list[str],
-    #     settings_class: type[JMinSettingsElectronic | JMinSettingsFluid | JMinSettingsIonic | JMinSettingsLattice],
-    # ) -> JMinSettingsElectronic | JMinSettingsFluid | JMinSettingsIonic | JMinSettingsLattice:
-    #     """Get appropriate JMinSettings mutant.
-
-    #     Get the settings object from the out file text.
-
-    #     Args:
-    #         text (list[str]): Output of read_file for out file.
-    #         settings_class (Type[JMinSettings]): Settings class to create object from.
-
-    #     Returns:
-    #         JMinSettingsElectronic | JMinSettingsFluid | JMinSettingsIonic | JMinSettingsLattice: Settings object.
-    #     """
-    #     settings_dict = self._create_settings_dict(text, settings_class.start_flag)
-    #     return settings_class(params=settings_dict) if len(settings_dict) else None
-
-    # def _set_min_settings(self, text: list[str]) -> None:
-    #     """Set the settings objects from the out file text.
-
-    #     Set the settings objects from the out file text.
-
-    #     Args:
-    #         text (list[str]): Output of read_file for out file.
-    #     """
-    #     self.jsettings_fluid = self._get_settings_object(text, JMinSettingsFluid)
-    #     self.jsettings_electronic = self._get_settings_object(text, JMinSettingsElectronic)
-    #     self.jsettings_lattice = self._get_settings_object(text, JMinSettingsLattice)
-    #     self.jsettings_ionic = self._get_settings_object(text, JMinSettingsIonic)
-    #     # if self.jsettings_lattice is not None and "niterations" in self.jsettings_lattice.params:
-    #     #     self.constant_lattice = int(self.jsettings_lattice.params["niterations"]) == 0
-
     def _set_geomopt_vars(self, text: list[str]) -> None:
         """Set the geom_opt and geom_opt_type class variables.
 
@@ -890,19 +856,6 @@ def _set_geomopt_vars(self, text: list[str]) -> None:
             self.geom_opt = False
             self.geom_opt_type = "single point"
             self.geom_opt_label = "IonicMinimize"
-        # # Attempts to set all self.jsettings_x class variables
-        # self._set_min_settings(text)
-        # if self.jsettings_ionic is None or self.jsettings_lattice is None:
-        #     raise ValueError("Unknown issue in setting settings objects")
-        # if int(self.jsettings_lattice.params["niterations"]) > 0:
-        #     self.geom_opt = True
-        #     self.geom_opt_type = "lattice"
-        # elif int(self.jsettings_ionic.params["niterations"]) > 0:
-        #     self.geom_opt = True
-        #     self.geom_opt_type = "ionic"
-        # else:
-        #     self.geom_opt = False
-        #     self.geom_opt_type = "single point"
 
     def _get_initial_structure(self, text: list[str]) -> Structure | None:
         """Get the initial structure from the out file text.
@@ -982,11 +935,6 @@ def _set_backup_vars(self, text: list[str]) -> None:
                 mval = float(_val)
             else:
                 mval = None
-            # val = None
-            # for line in lines[::-1]:
-            #     val = get_colon_val(text[line], "mu:")
-            #     if val is not None:
-            #         break
             self._mu_backup = mval
 
     def _set_orb_fillings_nobroad(self, nspin: float) -> None:

src/pymatgen/io/jdftx/outputs.py

@@ -559,9 +559,14 @@ def from_file(
         return cls(slices=slices)
 
     def __post_init__(self):
-        if len(self.slices):
+        last_slice = None
+        for slc in self.slices[::-1]:
+            if slc is not None:
+                last_slice = slc
+                break
+        if last_slice is not None:
             for var in _jof_atr_from_last_slice:
-                setattr(self, var, getattr(self.slices[-1], var))
+                setattr(self, var, getattr(last_slice, var))
             self.trajectory = self._get_trajectory()
 
     def _get_trajectory(self) -> Trajectory | None:

tests/io/jdftx/shared_test_utils.py

@@ -10,6 +10,7 @@
 from pathlib import Path
 
 import pytest
+from numpy import allclose, ndarray
 
 from pymatgen.util.testing import TEST_FILES_DIR
 
@@ -27,6 +28,8 @@ def assert_same_value(testval, knownval):
                 assert_same_value(testval[k], knownval[k])
         elif testval is None:
             assert knownval is None
+        elif isinstance(testval, ndarray):
+            assert allclose(testval, knownval)
         else:
             assert testval == knownval
     else:

tests/io/jdftx/test_jdftxoutfile.py

@@ -4,7 +4,9 @@
 
 import pytest
 
-from pymatgen.io.jdftx.outputs import JDFTXOutfile
+from pymatgen.io.jdftx._output_utils import read_outfile_slices
+from pymatgen.io.jdftx.jdftxoutfileslice import JDFTXOutfileSlice
+from pymatgen.io.jdftx.outputs import JDFTXOutfile, _jof_atr_from_last_slice
 
 from .outputs_test_utils import (
     etot_etype_outfile_known_simple,
@@ -27,6 +29,7 @@
     problem2_outfile_known_simple,
     problem2_outfile_path,
 )
+from .shared_test_utils import assert_same_value
 
 if TYPE_CHECKING:
     from pathlib import Path
@@ -71,3 +74,23 @@ def test_JDFTXOutfile_default_struc_inheritance(filename: Path, latknown: dict):
     for i in range(3):
         for j in range(3):
             assert pytest.approx(lattice[i][j]) == latknown[f"{i}{j}"]
+
+
+# Make sure all possible exceptions are caught when none_on_error is True
+@pytest.mark.parametrize(("ex_outfile_path"), [(partial_lattice_init_outfile_path)])
+def test_none_on_partial(ex_outfile_path: Path):
+    texts = read_outfile_slices(str(ex_outfile_path))
+    texts0 = texts[:-1]
+
+    slices = [
+        JDFTXOutfileSlice._from_out_slice(text, is_bgw=False, none_on_error=False) for i, text in enumerate(texts0)
+    ]
+    outfile1 = JDFTXOutfile(slices=slices)
+    slices.append(None)
+    outfile2 = JDFTXOutfile(slices=slices)
+    assert isinstance(outfile2, JDFTXOutfile)
+    for var in _jof_atr_from_last_slice:
+        assert_same_value(
+            getattr(outfile1, var),
+            getattr(outfile2, var),
+        )