Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Pint 0.24.4 / openff-units 0.3.0 #1990

Merged
merged 27 commits into from
Feb 13, 2025
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
705853c
Use Pint 0.24.4
mattwthompson Jan 2, 2025
8886200
Shim
mattwthompson Jan 2, 2025
cd1a7c4
Small reorg
mattwthompson Jan 2, 2025
6170e8c
Merge remote-tracking branch 'upstream/main' into pint-0.24.4
mattwthompson Feb 4, 2025
6a51948
Use possible 0.3.0
mattwthompson Feb 5, 2025
5cf8928
Try using Pint 0.24
mattwthompson Feb 5, 2025
6c60204
Debug
mattwthompson Feb 5, 2025
b8375fe
Debug
mattwthompson Feb 5, 2025
911593d
Be sad about typing regressions
mattwthompson Feb 5, 2025
d2ab0cf
Debug
mattwthompson Feb 5, 2025
880b653
Don't run examples
mattwthompson Feb 5, 2025
2a6350f
Debug
mattwthompson Feb 5, 2025
aa9c49e
Debug
mattwthompson Feb 5, 2025
f4eaef6
Debug
mattwthompson Feb 5, 2025
2504d7d
Do not trust `Unit.compatible_units`
mattwthompson Feb 5, 2025
082c7b6
Do not trust `compatible_units` in topology processing
mattwthompson Feb 5, 2025
563ae16
Fix
mattwthompson Feb 5, 2025
0bb8921
No NAGL
mattwthompson Feb 5, 2025
81bbed4
Remove shim
mattwthompson Feb 5, 2025
a1b6390
Revert "No NAGL"
mattwthompson Feb 5, 2025
8d72cc8
Revert "Don't run examples"
mattwthompson Feb 5, 2025
62b62ce
Test both "new" and "old" upstreams
mattwthompson Feb 7, 2025
3b4eb30
Merge branch 'main' into pint-0.24.4
j-wags Feb 13, 2025
6faac9f
Merge remote-tracking branch 'upstream/main' into pint-0.24.4
mattwthompson Feb 13, 2025
1efa553
Update release history
mattwthompson Feb 13, 2025
cd29a2d
Merge remote-tracking branch 'upstream/pint-0.24.4' into pint-0.24.4
mattwthompson Feb 13, 2025
a488524
Minor typo
mattwthompson Feb 13, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ defaults:
jobs:
test:
if: (github.event_name == 'schedule' && github.repository == 'openforcefield/openff-toolkit') || (github.event_name != 'schedule')
name: Test on ${{ matrix.os }}, Python ${{ matrix.python-version }}, RDKit=${{ matrix.rdkit }}, OpenEye=${{ matrix.openeye }}
name: Test on ${{ matrix.os }}, Python ${{ matrix.python-version }}, RDKit=${{ matrix.rdkit }}, OpenEye=${{ matrix.openeye }} "new Pint" {{ matrix.new-pint }}
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, macos-latest]
python-version: ["3.10", "3.11", "3.12"]
rdkit: [true, false]
openeye: [true, false]
new-pint: [false, true]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything touching this variable could be dropped before merge, but I think it's important to show changes with "new" and existing versions of the upsreams

exclude:
- rdkit: false
openeye: false
Expand Down Expand Up @@ -79,6 +79,13 @@ jobs:
OE_LICENSE_TEXT: ${{ secrets.OE_LICENSE }}
run: echo "${OE_LICENSE_TEXT}" > ${OE_LICENSE}

- name: Install Pint 0.24.4 and `openff-units` branch
if: matrix.new-pint
run: |
python -m pip install \
"pint==0.24.4" \
git+https://github.com/openforcefield/[email protected]

- name: Install package
run: |
# While Interchange is being installed with pip, there is no need to
Expand Down Expand Up @@ -145,7 +152,7 @@ jobs:
if [[ "$GITHUB_EVENT_NAME" == "schedule" ]]; then
PYTEST_ARGS+=" -m 'slow or not slow'"
fi

python -c "from pint import __version__; print(__version__)"
python -m pytest --durations=20 $PYTEST_ARGS $COV

- name: Run code snippets in docs
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/examples.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ jobs:
OE_LICENSE_TEXT: ${{ secrets.OE_LICENSE }}
run: echo "${OE_LICENSE_TEXT}" > ${OE_LICENSE}

- name: Install Pint 0.24.4 and `openff-units` branch
run: |
pip install "pint==0.24.4" git+https://github.com/openforcefield/[email protected]

- name: Install package
run: python -m pip install .

Expand Down
2 changes: 1 addition & 1 deletion devtools/conda-envs/openeye.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ dependencies:
- mdtraj
- nglview
- parmed =3
- mypy
- mypy <1.15
mattwthompson marked this conversation as resolved.
Show resolved Hide resolved
- typing_extensions
- pip:
- types-setuptools
Expand Down
2 changes: 1 addition & 1 deletion devtools/conda-envs/test_env.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ dependencies:
- openmm >=7.6
- openff-forcefields >=2023.05.1
- smirnoff99Frosst
- openff-units =0.2.0
- openff-units
- openff-amber-ff-ports
- openff-utilities >=0.1.5
- openff-interchange-base =0.4
Expand Down
3 changes: 1 addition & 2 deletions openff/toolkit/_tests/test_topology.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,10 @@
import numpy as np
import pytest
from openff.units.openmm import from_openmm
from openff.units.units import Quantity
from openff.utilities import skip_if_missing
from openmm import app

from openff.toolkit import unit
from openff.toolkit import Quantity, unit
from openff.toolkit._tests.create_molecules import (
create_ammonia,
create_cyclohexane,
Expand Down
29 changes: 15 additions & 14 deletions openff/toolkit/topology/molecule.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
)

import numpy as np
from openff.units import Unit
from openff.units.elements import MASSES, SYMBOLS
from openff.utilities.exceptions import MissingOptionalDependencyError
from typing_extensions import TypeAlias
Expand Down Expand Up @@ -110,6 +111,8 @@
B = TypeVar("B", bound="Bond")


_CHARGE_UNITS = set([Unit("elementary_charge")])

class MoleculeDeprecationWarning(UserWarning):
"""Warning for deprecated portions of the Molecule API."""

Expand Down Expand Up @@ -338,12 +341,10 @@ def formal_charge(self, other):
Set the atom's formal charge. Accepts either ints or unit-wrapped ints with units of charge.
"""
if isinstance(other, int):
self._formal_charge = Quantity(other, unit.elementary_charge)
self._formal_charge = Quantity(other, "elementary_charge")
elif isinstance(other, Quantity):
# Faster to check equality than convert, so short-circuit
if other.units is unit.elementary_charge:
self.formal_charge = other
elif other.units in unit.elementary_charge.compatible_units():
if other.units in _CHARGE_UNITS:
mattwthompson marked this conversation as resolved.
Show resolved Hide resolved
self._formal_charge = other
else:
raise IncompatibleUnitError(
Expand All @@ -361,7 +362,7 @@ def formal_charge(self, other):
from openff.units.openmm import from_openmm

converted = from_openmm(other)
if converted.units in unit.elementary_charge.compatible_units():
if converted.units in _CHARGE_UNITS:
self._formal_charge = converted
else:
raise IncompatibleUnitError(
Expand Down Expand Up @@ -3073,14 +3074,14 @@ def _add_conformer(self, coordinates: Quantity):
index
The index of this conformer
"""
if coordinates.shape != (self.n_atoms, 3):
if coordinates.shape != (self.n_atoms, 3): # type: ignore[attr-defined]
raise InvalidConformerError(
"molecule.add_conformer given input of the wrong shape: "
f"Given {coordinates.shape}, expected {(self.n_atoms, 3)}"
f"Given {coordinates.shape}, expected {(self.n_atoms, 3)}" # type: ignore[attr-defined]
)

if isinstance(coordinates, Quantity):
if not coordinates.units.is_compatible_with(unit.angstrom):
if not coordinates.units.is_compatible_with(unit.angstrom): # type: ignore[attr-defined]
raise IncompatibleUnitError(
"Coordinates passed to Molecule._add_conformer with incompatible units. "
"Ensure that units are dimension of length."
Expand Down Expand Up @@ -3115,7 +3116,7 @@ def _add_conformer(self, coordinates: Quantity):
np.zeros(shape=(self.n_atoms, 3), dtype=float), unit.angstrom
)
try:
tmp_conf[:] = coordinates
tmp_conf[:] = coordinates # type: ignore[index]
except AttributeError as e:
# TODO: Make this a warning, log it, or do something other than print
print(e)
Expand Down Expand Up @@ -3168,12 +3169,12 @@ def partial_charges(self, charges):
)

if isinstance(charges, Quantity):
if charges.units in unit.elementary_charge.compatible_units():
if charges.units in _CHARGE_UNITS:
self._partial_charges = charges.astype(float)
mattwthompson marked this conversation as resolved.
Show resolved Hide resolved
else:
raise IncompatibleUnitError(
"Unsupported unit passed to partial_charges setter. "
f"Found unit {charges.units}, expected {unit.elementary_charge}"
f"Found unit {charges.units}, expected elementary_charge"
)

elif hasattr(charges, "unit"):
Expand All @@ -3189,12 +3190,12 @@ def partial_charges(self, charges):
from openff.units.openmm import from_openmm

converted = from_openmm(charges)
if converted.units in unit.elementary_charge.compatible_units():
if converted.units in _CHARGE_UNITS:
self._partial_charges = converted.astype(float)
else:
raise IncompatibleUnitError(
"Unsupported unit passed to partial_charges setter. "
f"Found unit {converted.units}, expected {unit.elementary_charge}"
f"Found unit {converted.units}, expected elementary_charge"
)

else:
Expand Down Expand Up @@ -4108,7 +4109,7 @@ def title(frame):
# add the data to the xyz_data list
for i, geometry in enumerate(conformers, 1):
xyz_data.write(f"{self.n_atoms}\n" + title(end))
for j, atom_coords in enumerate(geometry.m_as(unit.angstrom)):
for j, atom_coords in enumerate(geometry.m_as(unit.angstrom)): # type: ignore[arg-type]
x, y, z = atom_coords
xyz_data.write(
f"{SYMBOLS[self.atoms[j].atomic_number]} {x: .10f} {y: .10f} {z: .10f}\n"
Expand Down
34 changes: 23 additions & 11 deletions openff/toolkit/topology/topology.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

import numpy as np
from numpy.typing import NDArray
from openff.units import Unit
from typing_extensions import TypeAlias

from openff.toolkit import Quantity, unit
Expand Down Expand Up @@ -76,6 +77,20 @@
TKR: TypeAlias = Union["ToolkitRegistry", "ToolkitWrapper"]
MoleculeLike: TypeAlias = Union["Molecule", "FrozenMolecule", "_SimpleMolecule"]

_DISTANCE_UNITS = set(
[
Unit(val) for val in
[
'nanometer',
'angstrom',
'micron',
'bohr',
'meter',
'fermi',
]
]
)


class _TransformedDict(MutableMapping):
"""A dictionary that transform and sort keys.
Expand Down Expand Up @@ -623,10 +638,7 @@ def box_vectors(self, box_vectors):
else:
raise InvalidBoxVectorsError("Given unitless box vectors")

# Unit.compatible_units() returns False with itself, for some reason
if (box_vectors.units != unit.nm) and (
box_vectors.units not in unit.nm.compatible_units()
):
if box_vectors.units not in _DISTANCE_UNITS:
raise InvalidBoxVectorsError(
f"Cannot set box vectors with quantities with unit {box_vectors.units}"
)
Expand Down Expand Up @@ -1236,7 +1248,7 @@ def to_dict(self) -> dict:
return_dict["aromaticity_model"] = self._aromaticity_model

return_dict["constrained_atom_pairs"] = {
constrained_atom_pair: quantity_to_string(distance)
constrained_atom_pair: quantity_to_string(distance) # type: ignore[arg-type]
for constrained_atom_pair, distance in self._constrained_atom_pairs.items()
}

Expand Down Expand Up @@ -1916,7 +1928,7 @@ def to_openmm(
(last_residue.name == atom_residue_name)
and (int(last_residue.id) == int(atom_residue_number))
and (last_residue.insertionCode == atom_insertion_code)
and (chain.id == last_chain.id)
and (chain.id == last_chain.id) # type: ignore[union-attr]
):
residue = last_residue
else:
Expand Down Expand Up @@ -2045,7 +2057,7 @@ def to_file(
if isinstance(positions, openmm.unit.Quantity):
openmm_positions: openmm.unit.Quantity = positions
elif isinstance(positions, Quantity):
openmm_positions = positions.to_openmm()
openmm_positions = positions.to_openmm() # type: ignore[attr-defined]
elif isinstance(positions, np.ndarray):
openmm_positions = openmm.unit.Quantity(positions, openmm.unit.angstroms)
elif positions is None:
Expand Down Expand Up @@ -2152,18 +2164,18 @@ def set_positions(self, array: Quantity):

# Copy the array in nanometers and make it an OpenFF Quantity
array = Quantity(np.asarray(array.to(unit.nanometer).magnitude), unit.nanometer)
if array.shape != (self.n_atoms, 3):
if array.shape != (self.n_atoms, 3): # type: ignore[attr-defined]
raise WrongShapeError(
f"Array has shape {array.shape} but should have shape {self.n_atoms, 3}"
f"Array has shape {array.shape} but should have shape {self.n_atoms, 3}" # type: ignore[attr-defined]
)

start = 0
for molecule in self.molecules:
stop = start + molecule.n_atoms
if molecule.conformers is None:
molecule._conformers = [array[start:stop]]
molecule._conformers = [array[start:stop]] # type: ignore[index]
else:
molecule.conformers[0:1] = [array[start:stop]]
molecule.conformers[0:1] = [array[start:stop]] # type: ignore[index]
start = stop

@classmethod
Expand Down
2 changes: 1 addition & 1 deletion openff/toolkit/typing/engines/smirnoff/forcefield.py
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,7 @@ def parse_sources(self, sources, allow_cosmetic_attributes: bool = True):
smirnoff_data, allow_cosmetic_attributes=allow_cosmetic_attributes
)

def _to_smirnoff_data(self, discard_cosmetic_attributes: bool = False) -> dict:
def _to_smirnoff_data(self, discard_cosmetic_attributes: bool = False) -> list | dict:
"""
Convert this ForceField and all related ParameterHandlers to a dict representing a SMIRNOFF
data object.
Expand Down
2 changes: 1 addition & 1 deletion openff/toolkit/typing/engines/smirnoff/parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ def _validate_units(attr, value: Union[str, Quantity], units: Unit):
value = object_to_quantity(value)

try:
if not units.is_compatible_with(value.units):
if not units.is_compatible_with(value.units): # type: ignore
raise IncompatibleUnitError(
f"{attr.name}={value} should have units of {units}"
)
Expand Down
7 changes: 3 additions & 4 deletions openff/toolkit/utils/ambertools_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,12 +265,11 @@ def assign_partial_charges(
with open(f"{tmpdir}/charges.txt") as infile:
contents = infile.read()
text_charges = contents.split()
charges = np.zeros([molecule.n_atoms], np.float64)
charges_array = np.zeros([molecule.n_atoms], np.float64)
for index, token in enumerate(text_charges):
charges[index] = float(token)
charges_array[index] = float(token)
# TODO: Ensure that the atoms in charged.mol2 are in the same order as in molecule.sdf
charges = Quantity(charges, unit.elementary_charge)
molecule.partial_charges = charges
molecule.partial_charges = Quantity(charges_array, unit.elementary_charge)

if normalize_partial_charges:
molecule._normalize_partial_charges()
Expand Down
2 changes: 1 addition & 1 deletion openff/toolkit/utils/builtin_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ def assign_partial_charges(
)
else:
mol_copy._conformers = None
for conformer in use_conformers:
for conformer in use_conformers: # type: ignore[attr-defined]
mol_copy._add_conformer(conformer)
self._check_n_conformers(
mol_copy,
Expand Down
21 changes: 11 additions & 10 deletions openff/toolkit/utils/openeye_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,10 @@

import numpy as np
from cachetools import LRUCache, cached
from openff.units.elements import SYMBOLS
from typing_extensions import TypeAlias

from openff.toolkit import Quantity, unit

if TYPE_CHECKING:
from openff.toolkit.topology.molecule import Atom, Bond, FrozenMolecule, Molecule

from openff.units.elements import SYMBOLS

from openff.toolkit.utils.base_wrapper import (
ToolkitWrapper,
_ChargeSettings,
Expand Down Expand Up @@ -64,6 +59,12 @@
)
from openff.toolkit.utils.utils import inherit_docstrings

if TYPE_CHECKING:
import openmm.app

from openff.toolkit.topology.molecule import Atom, Bond, FrozenMolecule, Molecule


logger = logging.getLogger(__name__)


Expand Down Expand Up @@ -332,8 +333,8 @@ def _polymer_openmm_topology_to_offmol(

def _polymer_openmm_topology_to_oemol(
self,
omm_top,
substructure_library,
omm_top: "openmm.app.Topology",
substructure_library: dict[str, dict],
):
"""
Parameters
Expand Down Expand Up @@ -1674,7 +1675,7 @@ def to_openeye(
# OE needs a 1 x (3*n_Atoms) double array as input
flat_coords = np.zeros(shape=oemol.NumAtoms() * 3, dtype=np.float64)
for index, oe_idx in off_to_oe_idx.items():
(x, y, z) = conf[index, :].m_as(unit.angstrom)
(x, y, z) = conf[index, :].m_as(unit.angstrom) # type: ignore[index]
flat_coords[(3 * oe_idx)] = x
flat_coords[(3 * oe_idx) + 1] = y
flat_coords[(3 * oe_idx) + 2] = z
Expand Down Expand Up @@ -2560,7 +2561,7 @@ def assign_partial_charges(
index = oeatom.GetIdx()
charge = oeatom.GetPartialCharge()
charge = charge * unit.elementary_charge
charges[index] = charge
charges[index] = charge # type: ignore[index]

molecule.partial_charges = charges

Expand Down
Loading