Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
d25a1a3
initial implementation of NAGLChargesHandler
j-wags Apr 23, 2025
10db658
have testing env use naglcharges interchange branch
j-wags Apr 23, 2025
5f02826
implement doi and hash fields
j-wags Jul 10, 2025
de0f3df
add tests
j-wags Jul 10, 2025
58e3335
fix tests
j-wags Jul 10, 2025
37b6417
add docstring
j-wags Jul 10, 2025
47496ae
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 10, 2025
831bc8b
force use of nagl for test
j-wags Jul 10, 2025
858d893
fix whitespace
j-wags Jul 10, 2025
4dfe8e1
add doi and file_hash to nagltoolkitwrapper.assign_partial_charges, s…
j-wags Jul 15, 2025
8153199
raise specific error for blank/none model_file values
j-wags Jul 15, 2025
d1878fb
improve docstring
j-wags Jul 15, 2025
787c089
test with new openff-nagl-models
j-wags Jul 15, 2025
5981249
update nagl test for bad suffix error
j-wags Jul 17, 2025
9b095f9
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 17, 2025
a16a426
update toolkit registry check to use types rather than repr
j-wags Jul 25, 2025
1788c64
add compatibility checks for doi and hash fields, add tests
j-wags Jul 29, 2025
706c84c
lint
j-wags Jul 29, 2025
3b5687f
move imports to top of file
j-wags Jul 29, 2025
0df39aa
thread naglcharges handler into inits for correct import paths and docs
j-wags Jul 29, 2025
800c2f1
remove commented code
j-wags Aug 7, 2025
ec518e7
have tests run without interchange branch
j-wags Aug 7, 2025
ae283a8
point to nagl-models main branch for pip installs
j-wags Aug 7, 2025
ad9b5a0
update expected error message in test
j-wags Aug 7, 2025
c4a370c
revert tests to using conda packages
j-wags Aug 14, 2025
62c0edb
revert other test envs
j-wags Aug 14, 2025
78cfcdd
Merge branch 'main' into naglcharges-handler
j-wags Aug 14, 2025
c37ab5a
Merge remote-tracking branch 'origin' into naglcharges-handler
j-wags Aug 14, 2025
fc431e2
update releasenotes
j-wags Aug 14, 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
2 changes: 2 additions & 0 deletions devtools/conda-envs/openeye-examples.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,5 @@ dependencies:
- pdbfixer
- openmmforcefields >=0.11.2
- gromacs >=2023.3
- pip:
- git+https://github.com/openforcefield/openff-interchange.git@naglcharges-handler
1 change: 1 addition & 0 deletions devtools/conda-envs/openeye.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,4 @@ dependencies:
- types-xmltodict
- types-cachetools
- mongo-types
- git+https://github.com/openforcefield/openff-interchange.git@naglcharges-handler
2 changes: 2 additions & 0 deletions devtools/conda-envs/rdkit-examples.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,5 @@ dependencies:
- pdbfixer
- openmmforcefields >=0.11.2
- gromacs >=2023.3
- pip:
- git+https://github.com/openforcefield/openff-interchange.git@naglcharges-handler
2 changes: 2 additions & 0 deletions devtools/conda-envs/rdkit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,5 @@ dependencies:
- qcportal >=0.50
- qcengine
- nglview
- pip:
- git+https://github.com/openforcefield/openff-interchange.git@naglcharges-handler
2 changes: 2 additions & 0 deletions devtools/conda-envs/test_env.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,5 @@ dependencies:
- nbval
# No idea why this is necessary, see https://github.com/openforcefield/openff-toolkit/pull/1821
- nomkl
- pip:
- git+https://github.com/openforcefield/openff-interchange.git@naglcharges-handler
7 changes: 6 additions & 1 deletion openff/toolkit/_tests/test_nagl.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
create_reversed_ethanol,
)
from openff.toolkit._tests.utils import requires_openeye
from openff.toolkit.utils import GLOBAL_TOOLKIT_REGISTRY
from openff.toolkit.utils.exceptions import (
ChargeMethodUnavailableError,
ToolkitUnavailableException,
Expand All @@ -38,6 +39,10 @@ def test_version(self):

assert parsed_version == NAGLToolkitWrapper()._toolkit_version

def test_nagl_in_global_toolkit_registry(self):
assert "NAGL" in GLOBAL_TOOLKIT_REGISTRY.__repr__()


@requires_openeye
@pytest.mark.parametrize(
"molecule_function",
Expand Down Expand Up @@ -66,7 +71,7 @@ def test_assign_partial_charges_basic(self, molecule_function, nagl_model):

molecule.assign_partial_charges(
partial_charge_method=nagl_model,
toolkit_registry=NAGLToolkitWrapper(),
#toolkit_registry=NAGLToolkitWrapper(),
)

assert molecule.partial_charges is not None
Expand Down
10 changes: 10 additions & 0 deletions openff/toolkit/_tests/test_parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -2722,6 +2722,16 @@ def test_charge_increment_one_ci_missing(self):
],
)

class TestNAGLChargesHandler:
def test_nagl_charges_handler_serialization(self):
from openff.toolkit.typing.engines.smirnoff import NAGLChargesHandler
handler = NAGLChargesHandler(model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt", skip_version_check=True)
assert handler.model_file == "openff-gnn-am1bcc-0.1.0-rc.3.pt"
handler_dict = handler.to_dict()
assert handler_dict["model_file"] == "openff-gnn-am1bcc-0.1.0-rc.3.pt"

# TODO: test_nagl_charges_handler_are_compatible

Copy link
Member

Choose a reason for hiding this comment

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

(certainly not blocking) the earlier tests looked at combinations of optional arguments missing but in ways that could be combined. A marginal add would be those cases in this test, but not particularly important


class TestGBSAHandler:
def test_create_default_gbsahandler(self):
Expand Down
1 change: 1 addition & 0 deletions openff/toolkit/typing/engines/smirnoff/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
IndexedParameterAttribute,
LibraryChargeHandler,
MappedParameterAttribute,
NAGLChargesHandler,
ParameterAttribute,
ParameterHandler,
ParameterList,
Expand Down
39 changes: 38 additions & 1 deletion openff/toolkit/typing/engines/smirnoff/parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"LibraryChargeHandler",
"LibraryChargeType",
"MappedParameterAttribute",
"NAGLChargesHandler",
"NotEnoughPointsForInterpolationError",
"ParameterAttribute",
"ParameterHandler",
Expand Down Expand Up @@ -3253,6 +3254,41 @@ def find_matches(self, entity, unique=False):
unique=unique,
)

class NAGLChargesHandler(_NonbondedHandler):
"""ParameterHandler for applying partial charges from a pretrained NAGL model."""

_TAGNAME = "NAGLCharges"
_DEPENDENCIES = [vdWHandler, ElectrostaticsHandler, LibraryChargeHandler]
_INFOTYPE = None # No separate parameter types; just a model path
_MAX_SUPPORTED_SECTION_VERSION = Version("0.3")
model_file = ParameterAttribute(converter=str)

def check_handler_compatibility(
self,
other_handler: "NAGLChargesHandler",
assume_missing_is_default: bool = True,
):
"""
Checks whether this ParameterHandler encodes compatible physics as another ParameterHandler. This is
called if a second handler is attempted to be initialized for the same tag.

Parameters
----------
other_handler
The handler to compare to.
assume_missing_is_default

Raises
------
IncompatibleParameterError if handler_kwargs are incompatible with existing parameters.
"""
if self.model_file != other_handler.model_file:
raise IncompatibleParameterError("Attempted to initialize two NAGLCharges sections with different "
"model_files: "
f"{self.model_file=} is not identical to {other_handler.model_file=}")




class ToolkitAM1BCCHandler(_NonbondedHandler):
"""Handle SMIRNOFF ``<ToolkitAM1BCC>`` tags
Expand All @@ -3261,7 +3297,7 @@ class ToolkitAM1BCCHandler(_NonbondedHandler):
"""

_TAGNAME = "ToolkitAM1BCC" # SMIRNOFF tag name to process
_DEPENDENCIES = [vdWHandler, ElectrostaticsHandler, LibraryChargeHandler]
Copy link
Member

Choose a reason for hiding this comment

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

For better or worse, Interchange does not use this and I don't think anything else does

_DEPENDENCIES = [vdWHandler, ElectrostaticsHandler, LibraryChargeHandler, NAGLChargesHandler]
_KWARGS = ["toolkit_registry"] # Kwargs to catch when create_force is called

def check_handler_compatibility(
Expand Down Expand Up @@ -3382,6 +3418,7 @@ def find_matches(self, entity, unique=False):
return matches



class GBSAHandler(ParameterHandler):
"""Handle SMIRNOFF ``<GBSA>`` tags

Expand Down
2 changes: 1 addition & 1 deletion openff/toolkit/utils/nagl_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def assign_partial_charges(
except FileNotFoundError as error:
raise ChargeMethodUnavailableError(
f"Charge model {partial_charge_method} not supported by "
f"{self.__class__.__name__}."
f"{self.__class__.__name__}, or model file can not be found."
) from error

model = GNNModel.load(model_path, eval_mode=True)
Expand Down
1 change: 1 addition & 0 deletions openff/toolkit/utils/toolkits.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@
# Create global toolkit registry, where all available toolkits are registered
GLOBAL_TOOLKIT_REGISTRY = ToolkitRegistry(
toolkit_precedence=[
NAGLToolkitWrapper,
OpenEyeToolkitWrapper,
RDKitToolkitWrapper,
AmberToolsToolkitWrapper,
Expand Down