From 97af5935b040556903d1056278a7eff3efe24c01 Mon Sep 17 00:00:00 2001 From: Matt Thompson Date: Sat, 3 Feb 2024 10:32:35 -0600 Subject: [PATCH] Interoperability trunk (#1818) * update SimpleMolecule to have has_unique_atom_names * update topology copy initializer to work with live objects * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Test re-initializing mixed topologies * clean up SimpleMolecule.has_unique_atom_names and add test * clean up unused lines in topology.py * Allow `_SimpleAtom.name` * Dev * Revert dev changes * Simplify `_SimpleAtom.has_unique_atom_names` * Make _SimpleMolecule._from_subgraph bring hierarchy info through * update releasehistory * pin away from openeye package that thinks our license is invalid in CI * Unpin OETK in test env (I updated the license file in GH secrets during this commit) * try running CI with nomkl --------- Co-authored-by: Jeffrey Wagner Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- devtools/conda-envs/test_env.yaml | 2 ++ docs/releasehistory.md | 11 ++---- openff/toolkit/_tests/test_mm_molecule.py | 20 +++++++---- openff/toolkit/_tests/test_topology.py | 14 ++++++++ openff/toolkit/topology/_mm_molecule.py | 43 +++++++++++++++++++++-- openff/toolkit/topology/molecule.py | 4 ++- openff/toolkit/topology/topology.py | 9 +++-- 7 files changed, 84 insertions(+), 19 deletions(-) diff --git a/devtools/conda-envs/test_env.yaml b/devtools/conda-envs/test_env.yaml index c5c083f7d..3f83bb2a0 100644 --- a/devtools/conda-envs/test_env.yaml +++ b/devtools/conda-envs/test_env.yaml @@ -41,6 +41,8 @@ dependencies: - nglview - mdtraj - nbval + # No idea why this is necessary, see https://github.com/openforcefield/openff-toolkit/pull/1821 + - nomkl - mypy - typing_extensions - pydantic =1 diff --git a/docs/releasehistory.md b/docs/releasehistory.md index aa8bba643..1732e28ee 100644 --- a/docs/releasehistory.md +++ b/docs/releasehistory.md @@ -6,17 +6,12 @@ Releases follow the `major.minor.micro` scheme recommended by [PEP440](https://w * `minor` increments add features but do not break API compatibility * `micro` increments represent bugfix releases or improvements in documentation -## Current development - -### API-breaking changes - -### Behavior changes - -### Bugfixes +## 0.15.2 ### New features -### Improved documentation and warnings +- [PR #1818](https://github.com/openforcefield/openff-toolkit/pull/1818): Several improvements to enable the practical use of ``Interchange.from_openmm``. + ## 0.15.1 diff --git a/openff/toolkit/_tests/test_mm_molecule.py b/openff/toolkit/_tests/test_mm_molecule.py index 9a3befed2..f1da8bce6 100644 --- a/openff/toolkit/_tests/test_mm_molecule.py +++ b/openff/toolkit/_tests/test_mm_molecule.py @@ -215,9 +215,6 @@ def test_to_t4_topology(self, t4): assert topology.n_atoms == t4.n_atoms assert topology.n_bonds == t4.n_bonds - @pytest.mark.skip( - reason="Fails because of https://github.com/openforcefield/openff-toolkit/issues/1783" - ) def test_to_openmm_topology(self, methanol): topology = methanol.to_topology().to_openmm() @@ -234,9 +231,20 @@ def test_to_openmm_topology_t4(self, t4): assert topology.getNumAtoms() == t4.n_atoms assert topology.getNumBonds() == t4.n_bonds - @pytest.mark.skip(reason="Not written") - def test_generate_unique_atom_names(self): - pass + def test_generate_unique_atom_names(self, water): + + # Initially atom names default to empty string + assert hasattr(water.atom(0), "name") + assert water.atom(0).name == "" + + assert water.has_unique_atom_names is False + # Assign unique atom names and ensure that they're unique + water.generate_unique_atom_names() + assert water.has_unique_atom_names is True + # Now manually make them not-unique and ensure that has_unique_atom_names reflects that + water.atom(0).name = "foo" + water.atom(1).name = "foo" + assert water.has_unique_atom_names is False class TestImpropers: diff --git a/openff/toolkit/_tests/test_topology.py b/openff/toolkit/_tests/test_topology.py index 09d2f331d..4ba5904a4 100644 --- a/openff/toolkit/_tests/test_topology.py +++ b/openff/toolkit/_tests/test_topology.py @@ -2459,3 +2459,17 @@ def test_symmetrical_group_ARG(self): assert sorted(sym_atoms) == [3, 4, 10, 13] assert sorted(sym_bonds) == [(2, 3), (2, 4), (9, 10), (9, 13)] + + +class TestMixedTopology: + def test_reinitialized_mixed_topology(self, mixed_topology): + copied = Topology(mixed_topology) + + assert copied.n_atoms == mixed_topology.n_atoms + assert copied.n_bonds == mixed_topology.n_bonds + assert copied.n_molecules == mixed_topology.n_molecules + + assert ( + copied.atom(int(copied.n_atoms / 2)).atomic_number + == mixed_topology.atom(int(mixed_topology.n_atoms / 2)).atomic_number + ) diff --git a/openff/toolkit/topology/_mm_molecule.py b/openff/toolkit/topology/_mm_molecule.py index 5df0866ca..495a4610e 100644 --- a/openff/toolkit/topology/_mm_molecule.py +++ b/openff/toolkit/topology/_mm_molecule.py @@ -21,6 +21,7 @@ HierarchyScheme, Molecule, _atom_nums_to_hill_formula, + _has_unique_atom_names, ) from openff.toolkit.utils.exceptions import UnsupportedMoleculeConversionError from openff.toolkit.utils.utils import deserialize_numpy, serialize_numpy @@ -282,7 +283,22 @@ def _from_subgraph(cls, subgraph: "nx.Graph"): offset = min(subgraph.nodes()) for _, node_data in subgraph.nodes(data=True): - molecule.add_atom(atomic_number=node_data["atomic_number"]) + # Hierarchy metadata like residue name needs to be passed in as a separate argument, + # so we extract those values from the node data + metadata = dict() + for key, val in node_data.items(): + if key in [ + "residue_name", + "residue_number", + "insertion_code", + "chain_id", + ]: + metadata[key] = val + # Then we remove the metadata items that we took from the node data + for key in metadata.keys(): + del node_data[key] + + molecule.add_atom(metadata=metadata, **node_data) for topology_index1, topology_index2, _edge_data in subgraph.edges(data=True): molecule.add_bond(topology_index1 - offset, topology_index2 - offset) @@ -494,6 +510,11 @@ def generate_unique_atom_names(self): atom.name = symbol + str(element_counts[symbol]) + "x" + @property + def has_unique_atom_names(self) -> bool: + """``True`` if the molecule has unique atom names, ``False`` otherwise.""" + return _has_unique_atom_names(self) + def __getattr__(self, name: str) -> list["HierarchyElement"]: """If a requested attribute is not found, check the hierarchy schemes""" try: @@ -512,17 +533,35 @@ def __deepcopy__(self, memo): class _SimpleAtom: - def __init__(self, atomic_number: int, molecule=None, metadata=None, **kwargs): + def __init__( + self, + atomic_number: int, + molecule: Optional[_SimpleMolecule] = None, + metadata: Optional[AtomMetadataDict] = None, + name: str = "", + **kwargs, + ): if metadata is None: self.metadata = AtomMetadataDict() else: self.metadata = AtomMetadataDict(metadata) + + # This _could_ be hidden away into _metadata + self._name = name self._atomic_number = atomic_number self._molecule = molecule self._bonds: list[Optional[_SimpleBond]] = list() for key, val in kwargs.items(): setattr(self, key, val) + @property + def name(self) -> str: + return self._name + + @name.setter + def name(self, value: str): + self._name = value + @property def atomic_number(self) -> int: return self._atomic_number diff --git a/openff/toolkit/topology/molecule.py b/openff/toolkit/topology/molecule.py index 5244fc3ab..6d0ae99b3 100644 --- a/openff/toolkit/topology/molecule.py +++ b/openff/toolkit/topology/molecule.py @@ -6100,7 +6100,9 @@ def generate_unique_atom_names(self): return _generate_unique_atom_names(self) -def _has_unique_atom_names(obj: Union[FrozenMolecule, HierarchyElement]) -> bool: +def _has_unique_atom_names( + obj: Union[FrozenMolecule, "_SimpleMolecule", HierarchyElement] +) -> bool: """``True`` if the object has unique atom names, ``False`` otherwise.""" unique_atom_names = set([atom.name for atom in obj.atoms]) if len(unique_atom_names) < obj.n_atoms: diff --git a/openff/toolkit/topology/topology.py b/openff/toolkit/topology/topology.py index 42a7d240e..5ed376d60 100644 --- a/openff/toolkit/topology/topology.py +++ b/openff/toolkit/topology/topology.py @@ -1205,8 +1205,13 @@ def _invalidate_cached_properties(self): del atom.__dict__["_topology_atom_index"] def copy_initializer(self, other): - other_dict = deepcopy(other.to_dict()) - self._initialize_from_dict(other_dict) + import copy + + self.aromaticity_model = other.aromaticity_model + self._constrained_atom_pairs = copy.deepcopy(other._constrained_atom_pairs) + self._box_vectors = copy.deepcopy(other._box_vectors) + self._molecules = copy.deepcopy(other._molecules) + self._invalidate_cached_properties() def to_dict(self): from openff.toolkit.utils.utils import serialize_numpy