Skip to content

Commit

Permalink
Interoperability trunk (#1818)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored Feb 3, 2024
1 parent d0e63b9 commit 97af593
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 19 deletions.
2 changes: 2 additions & 0 deletions devtools/conda-envs/test_env.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 3 additions & 8 deletions docs/releasehistory.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
20 changes: 14 additions & 6 deletions openff/toolkit/_tests/test_mm_molecule.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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:
Expand Down
14 changes: 14 additions & 0 deletions openff/toolkit/_tests/test_topology.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
43 changes: 41 additions & 2 deletions openff/toolkit/topology/_mm_molecule.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand Down
4 changes: 3 additions & 1 deletion openff/toolkit/topology/molecule.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
9 changes: 7 additions & 2 deletions openff/toolkit/topology/topology.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 97af593

Please sign in to comment.