Refactor FORCECONSTANTS validation logic, add unit tests

Author: ajjackson

Testing/Data/UnitTest/Ge-force_constants.hdf5.md5

@@ -0,0 +1 @@
+ce75e1d1d4e693a320349b2e996f6d4a

Testing/Data/UnitTest/Ge-phonopy.yml.md5

@@ -0,0 +1 @@
+5e3f4a778792461cfa47d5b5d1003309

scripts/abins/abinsalgorithm.py

@@ -840,39 +840,45 @@ def _validate_euphonic_input_file(cls, filename_full_path: str) -> dict:
 
         if (suffix := path.suffix) == ".castep_bin":
             # Assume any .castep_bin file is valid choice
-            pass
-
-        elif suffix in (".yaml", ".yml"):
-            # Check .yaml files have expected keys for Phonopy force constants
-            with open(filename_full_path, "r") as yaml_file:
-                phonon_data = yaml.load(yaml_file, Loader=SafeLoader)
-
-            if {"phonopy", "force_constants"}.issubset(phonon_data):
-                pass
-
-            elif "phonopy" in phonon_data:
-                # Phonopy file without force constants: they must be in another file
-
-                # Check if following janus conventions:
-                # /parent/seedname-phonopy.yml -> /parent/seedname-force_constants.hdf5
-                janus_phonopy_re = "(?P<seedname>.+)-phonopy.yml"
-                fc_filenames = ("FORCE_CONSTANTS", "force_constants.hdf5")
-                if re_match := re.match(janus_phonopy_re, path.name):
-                    fc_file = path.parent / f"{re_match['seedname']}-force_constants.hdf5"
-                    if not fc_file.is_file():
-                        return dict(
-                            Invalid=True,
-                            Comment=f"Could not find force constants in {filename_full_path}, or find data file {fc_file}",
-                        )
+            return dict(Invalid=False, Comment="")
 
-                # Otherwise FC could be in a FORCE_CONSTANTS or force_constants.hdf5 file
-                elif not any(map(lambda fc_filename: (path.parent / fc_filename).is_file(), fc_filenames)):
-                    return dict(
-                        Invalid=True,
-                        Comment=f"Could not find force constants in {filename_full_path}, or find data file {' or '.join(fc_filenames)}",
-                    )
+        if suffix not in (".yaml", ".yml"):
+            return dict(Invalid=True, Comment="Invalid extension: FORCECONSTANTS requires .castep_bin, .yaml or .yml")
+
+        # Check .yaml files have expected keys for Phonopy force constants
+        with open(filename_full_path, "r") as yaml_file:
+            phonon_data = yaml.load(yaml_file, Loader=SafeLoader)
+
+        if {"phonopy", "force_constants"}.issubset(phonon_data):
+            # Force constants are in the yaml file: good
+            return dict(Invalid=False, Comment="")
+
+        if "phonopy" not in phonon_data:
+            # Not a Phonopy file: can't use this
+            return dict(Invalid=True, Comment=f"No 'phonopy' section found in {filename_full_path}")
+
+        # Phonopy file without force constants: they must be in another file
+
+        # Check if following janus conventions:
+        # /parent/seedname-phonopy.yml -> /parent/seedname-force_constants.hdf5
+        janus_phonopy_re = "(?P<seedname>.+)-phonopy.yml"
+        fc_filenames = ("FORCE_CONSTANTS", "force_constants.hdf5")
+        if re_match := re.match(janus_phonopy_re, path.name):
+            fc_file = path.parent / f"{re_match['seedname']}-force_constants.hdf5"
+            if not fc_file.is_file():
+                return dict(
+                    Invalid=True,
+                    Comment=f"Could not find force constants in {filename_full_path}, or find data file {fc_file}",
+                )
+
+        # Otherwise FC could be in a FORCE_CONSTANTS or force_constants.hdf5 file
+        elif not any(map(lambda fc_filename: (path.parent / fc_filename).is_file(), fc_filenames)):
+            return dict(
+                Invalid=True,
+                Comment=f"Could not find force constants in {filename_full_path}, or find data file {' or '.join(fc_filenames)}",
+            )
 
-        # Did not return already: No problems found
+        # Phonopy YAML with available force constants
         return dict(Invalid=False, Comment="")
 
     @classmethod

scripts/test/Abins/AbinsAlgorithmTest.py

@@ -42,6 +42,30 @@ def test_bad_atom_info(self):
             species.neutron_data
 
 
+class AbinsAlgorithmValidatorsTest(unittest.TestCase):
+    """Test input validators"""
+
+    def test_validate_euphonic_yml(self):
+        """Check that force constants are located for .yml input"""
+        # with open(abins.test_helpers.find_file(seedname + "_data.txt")) as data_file:
+        # def _validate_euphonic_input_file(cls, filename_full_path: str) -> dict:
+        from abins.test_helpers import find_file
+
+        issues = AbinsAlgorithm._validate_euphonic_input_file(find_file("Al_LoadPhonopy.yaml"))
+        self.assertEqual(issues, {"Invalid": False, "Comment": ""})
+
+        issues = AbinsAlgorithm._validate_euphonic_input_file(find_file("Ge-phonopy.yml"))
+        self.assertEqual(issues, {"Invalid": False, "Comment": ""})
+
+        issues = AbinsAlgorithm._validate_euphonic_input_file(find_file("Si2-phonon_LoadCASTEP.phonon"))
+        self.assertEqual(issues, {"Invalid": True, "Comment": "Invalid extension: FORCECONSTANTS requires .castep_bin, .yaml or .yml"})
+
+        # Non-phonopy yaml from another test
+        wrong_yaml = find_file("ISISPowderRunDetailsTestCallable.yaml")
+        issues = AbinsAlgorithm._validate_euphonic_input_file(wrong_yaml)
+        self.assertEqual(issues, {"Invalid": True, "Comment": f"No 'phonopy' section found in {wrong_yaml}"})
+
+
 class AbinsAlgorithmMethodsTest(unittest.TestCase):
     """Test static methods on AbinsAlgorithm"""