Merge pull request #277 from ISISNeutronMuon/chi/mdanse-unittest-fixes

MDANSE UnitTest fixes

.github/workflows/tests.yml

@@ -0,0 +1,36 @@
+name: Test MDANSE
+
+on: [push, pull_request]
+
+jobs:
+  test:
+    name: Build MDANSE and run tests on ${{ matrix.os }}
+    runs-on: ${{ matrix.os }}
+    strategy:
+      fail-fast: false
+      matrix:
+        os: [ubuntu-20.04, windows-2019, macOS-11]
+        python-version: ["3.9", "3.10", "3.11"]
+        # os: [ubuntu-20.04, windows-2019, macOS-11, ubuntu-latest, windows-latest, macOS-latest]
+    if: |
+      !contains( github.ref, 'legacy' )
+    steps:
+      - uses: actions/checkout@v3
+
+      - uses: actions/setup-python@v3
+        with:
+          python-version: ${{ matrix.python-version }}
+
+      - name: Install build packages
+        run: python -m pip install build wheel setuptools pytest
+
+      - name: Install dependencies
+        run: (cd MDANSE && python -m pip install -r requirements.txt)
+
+      - name: install MDANSE
+        run: (cd MDANSE && python -m pip install .)
+
+      - name: run unit tests
+        run: |
+          cd MDANSE/Tests/UnitTests
+          python -m pytest

.github/workflows/wheels.yml

@@ -10,7 +10,7 @@ jobs:
       fail-fast: false
       matrix:
         os: [ubuntu-20.04, windows-2019, macOS-11]
-        python-version: ["3.8", "3.9", "3.10", "3.11"]
+        python-version: ["3.9", "3.10", "3.11"]
         # os: [ubuntu-20.04, windows-2019, macOS-11, ubuntu-latest, windows-latest, macOS-latest]
     if: |
       !contains( github.ref, 'legacy' )

MDANSE/Extensions/contiguous_coordinates.pyx

@@ -262,17 +262,17 @@ def continuous_coordinates(
     sys.setrecursionlimit(100000)
 
     # Retrieve the top level chemical entities to which belong each of the selected atom
-    atoms = chemical_system.atom_list()
+    atoms = chemical_system.atom_list
     if selected_indexes is None:
         selected_indexes = [at.index for at in atoms]
 
-    chemical_entities = set([atoms[idx].top_level_chemical_entity() for idx in selected_indexes])
+    chemical_entities = set([atoms[idx].top_level_chemical_entity for idx in selected_indexes])
 
     # Set the bond network for these chemical entities
     bonds = {}
     chemical_entities_indexes = []
     for ce in chemical_entities:
-        for at in ce.atom_list():
+        for at in ce.atom_list:
             bonds[at.index] = [bat.index for bat in at.bonds]
             chemical_entities_indexes.append(at.index)
 

MDANSE/MANIFEST.in

@@ -5,6 +5,7 @@ recursive-include Extensions/qhull_lib *.h *c *.pxd
 recursive-include Extensions/xtc *.h *c *.pxd
 recursive-include Src/MDANSE/Chemistry *.json
 recursive-include Src/MDANSE/Framework *.json
+recursive-include Tests *.pdb *.h5
 
 recursive-include Doc *
 

MDANSE/Src/MDANSE/Chemistry/ChemicalEntity.py

@@ -475,6 +475,8 @@ def __init__(
 
         self._parent = None
 
+        self.element = ATOMS_DATABASE[self._symbol]["element"]
+
         for k, v in kwargs.items():
             try:
                 setattr(self, k, v)
@@ -518,11 +520,13 @@ def restore_bonds(self, atom_dict: dict[str, Atom]) -> List[Tuple[int, int]]:
             atom_dict -- dictionary of str: atom pairs,
                 where the key is the name of the Atom instance
         """
-        new_bonds = [atom_dict[name] for name in self._bonds]
+        new_bonds = [atom_dict[atm.name] for atm in self._bonds]
         self._bonds = new_bonds
         return [(self.index, other.index) for other in self._bonds]
 
     def __eq__(self, other):
+        if not isinstance(other, Atom):
+            return False
         if not self._index == other._index:
             return False
         if not self._symbol == other._symbol:
@@ -1061,12 +1065,14 @@ def build(
         mol = cls(code, name)
         contents = h5_contents["atoms"]
 
-        names = [literal_eval(contents[index][1]) for index in atom_indexes]
+        names = [
+            literal_eval(contents[index][1].decode("utf8")) for index in atom_indexes
+        ]
 
         mol.reorder_atoms(names)
 
         for at, index in zip(mol.atom_list, atom_indexes):
-            at.ghost = literal_eval(contents[index][2])
+            at.ghost = literal_eval(contents[index][2].decode("utf8"))
 
         return mol
 
@@ -1303,7 +1309,10 @@ def build(
         """
         res = cls(code, name, variant)
 
-        names = [literal_eval(h5_contents["atoms"][index][1]) for index in atom_indexes]
+        names = [
+            literal_eval(h5_contents["atoms"][index][1].decode("utf8"))
+            for index in atom_indexes
+        ]
         res.set_atoms(names)
 
         return res
@@ -1528,7 +1537,10 @@ def build(
         """
         nucl = cls(code, name, variant)
 
-        names = [literal_eval(h5_contents["atoms"][index][1]) for index in atom_indexes]
+        names = [
+            literal_eval(h5_contents["atoms"][index][1].decode("utf8"))
+            for index in atom_indexes
+        ]
         nucl.set_atoms(names)
 
         return nucl
@@ -1823,6 +1835,10 @@ def serialize(self, h5_contents: dict[str, list[list[str]]]) -> tuple[str, int]:
 
         return "nucleotide_chains", len(h5_contents["nucleotide_chains"]) - 1
 
+    @property
+    def nucleotides(self):
+        return self._nucleotides
+
     def set_nucleotides(self, nucleotides: list[Nucleotide]) -> None:
         """
         Sets the provided Nucleotide objects as the nucleotides making up this chain, and creates bonds between them.

MDANSE/Src/MDANSE/Framework/Jobs/IJob.py

@@ -315,7 +315,7 @@ def run(self, parameters, status=False):
         """
 
         try:
-            self._name = "%s_%s" % (self.__class__, IJob.define_unique_name())
+            self._name = "%s_%s" % (self.__class__.__name__, IJob.define_unique_name())
 
             if status and self._status is None:
                 self._status = self._status_constructor(self)

MDANSE/Src/MDANSE/Framework/Selectors/Macromolecule.py

@@ -55,5 +55,10 @@ def select(self, macromolecules):
                 m = Macromolecule.lookup.get(ce.__class__, None)
                 if m in macromolecules:
                     sel.update([at for at in ce.atom_list])
+                if isinstance(ce, Protein) and "peptide_chain" in macromolecules:
+                    for ce2 in ce.peptide_chains:
+                        m = Macromolecule.lookup.get(ce2.__class__, None)
+                        if m in macromolecules:
+                            sel.update([at for at in ce.atom_list])
 
         return sel

MDANSE/Src/MDANSE/Framework/Units.py

@@ -161,7 +161,7 @@ def __add__(self, other):
         else:
             raise UnitError("Incompatible units")
 
-    def __div__(self, other):
+    def __truediv__(self, other):
         """Divide _Unit instances.
 
         >>> print(measure(100, 'V') / measure(10, 'kohm'))

MDANSE/Src/MDANSE/Mathematics/LinearAlgebra.py

@@ -352,7 +352,7 @@ def asRotation(self):
         @rtype: L{Scientific.Geometry.Transformation.Rotation}
         @raises ValueError: if the quaternion is not normalized
         """
-        from Transformation import Rotation
+        from MDANSE.Mathematics.Transformation import Rotation
 
         if np.fabs(self.norm() - 1.0) > 1.0e-5:
             raise ValueError("Quaternion not normalized")

MDANSE/Src/MDANSE/MolecularDynamics/Configuration.py

@@ -37,10 +37,13 @@ def __init__(self, chemical_system: ChemicalSystem, coords: ArrayLike, **variabl
 
         self._variables = {}
 
-        self["coordinates"] = np.array(coords)
+        self["coordinates"] = np.array(coords, dtype=float)
 
         for k, v in variables.items():
-            self[k] = v
+            if k == "velocities" or k == "forces":
+                self[k] = np.array(v, dtype=float)
+            else:
+                self[k] = v
 
     def __contains__(self, item: str) -> bool:
         """
@@ -76,11 +79,22 @@ def __setitem__(self, name: str, value: ArrayLike) -> None:
         :param value: the value of the variable to be set
         :type value: numpy.ndarray
         """
-
         item = np.array(value)
+
+        if name == "unit_cell":
+            if item.shape != (3, 3):
+                raise ValueError(
+                    f"Invalid item dimensions for {name}; a shape of (3, 3) "
+                    f"was expected but data with shape of {item.shape} was "
+                    f"provided."
+                )
+            else:
+                self._variables[name] = value
+                return
+
         if item.shape != (self._chemical_system.number_of_atoms, 3):
             raise ValueError(
-                f"Invalid item dimensions; a shape of {(self._chemical_system.number_of_atoms, 3)} was "
+                f"Invalid item dimensions for {name}; a shape of {(self._chemical_system.number_of_atoms, 3)} was "
                 f"expected but data with shape of {item.shape} was provided."
             )
 
@@ -719,13 +733,13 @@ def contiguous_offsets(
         if chemical_entities is None:
             chemical_entities = self._chemical_system.chemical_entities
         else:
-            for ce in chemical_entities:
+            for j, ce in enumerate(chemical_entities):
                 root = ce.root_chemical_system
                 if root is not self._chemical_system:
                     raise ConfigurationError(
                         "All the entities provided in the chemical_entities parameter must belong "
                         "to the chemical system registered with this configuration, which is "
-                        f"{self._chemical_system.name}. However, the entity at index {i}, "
+                        f"{self._chemical_system.name}. However, the entity at index {j}, "
                         f"{str(ce)}, is in chemical system "
                         f"{root.name if root is not None else None}.\nExpected chemical system: "
                         f"{repr(self._chemical_system)}\nActual chemical system: {repr(root)}\n"

MDANSE/Src/MDANSE/MolecularDynamics/Trajectory.py

@@ -97,11 +97,14 @@ def __getitem__(self, frame):
         """
 
         grp = self._h5_file["/configuration"]
-
         configuration = {}
         for k, v in grp.items():
             configuration[k] = v[frame].astype(np.float64)
 
+        for k in self._h5_file.keys():
+            if k in ("time", "unit_cell"):
+                configuration[k] = self._h5_file[k][frame].astype(np.float64)
+
         return configuration
 
     def __getstate__(self):
@@ -229,7 +232,6 @@ def read_com_trajectory(
 
         indexes = [at.index for at in atoms]
         masses = np.array([ATOMS_DATABASE[at.symbol]["atomic_weight"] for at in atoms])
-
         grp = self._h5_file["/configuration"]
 
         coords = grp["coordinates"][first:last:step, :, :].astype(np.float64)
@@ -250,7 +252,7 @@ def read_com_trajectory(
             bonds = {}
             for e in top_lvl_chemical_entities:
                 for at in e.atom_list:
-                    bonds[at.index] = [idx for idx in at.bonds]
+                    bonds[at.index] = [other_at.index for other_at in at.bonds]
 
             com_traj = com_trajectory.com_trajectory(
                 coords,
@@ -444,7 +446,7 @@ def __init__(
 
         self._h5_file = h5py.File(self._h5_filename, "w")
 
-        self._chemical_system = chemical_system.copy()
+        self._chemical_system = chemical_system
 
         if selected_atoms is None:
             self._selected_atoms = self._chemical_system.atom_list

MDANSE/Src/MDANSE/MolecularDynamics/TrajectoryUtils.py

@@ -212,7 +212,7 @@ def build_connectivity(
         elif isinstance(ce, AtomCluster):
             atom_clusters.append(ce)
         else:
-            if ce.number_of_atoms() == 1:
+            if ce.number_of_atoms == 1:
                 single_atoms_objects.extend(ce.atom_list)
 
     if single_atoms_objects: