Skip to content

Commit d70974d

Browse files
j-wagspre-commit-ci[bot]
j-wags
and
pre-commit-ci[bot]
authored
Implement NAGLChargesHandler (#2048)
* initial implementation of NAGLChargesHandler * have testing env use naglcharges interchange branch * implement doi and hash fields * add tests * fix tests * add docstring * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * force use of nagl for test * fix whitespace * add doi and file_hash to nagltoolkitwrapper.assign_partial_charges, switch to use nagl_models' get_model method * raise specific error for blank/none model_file values * improve docstring * test with new openff-nagl-models * update nagl test for bad suffix error * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * update toolkit registry check to use types rather than repr * add compatibility checks for doi and hash fields, add tests * lint * move imports to top of file * thread naglcharges handler into inits for correct import paths and docs * remove commented code * have tests run without interchange branch * point to nagl-models main branch for pip installs * update expected error message in test * revert tests to using conda packages * revert other test envs * update releasenotes --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
1 parent 143a70f commit d70974d

File tree

9 files changed

+342
-14
lines changed

9 files changed

+342
-14
lines changed

‎docs/releasehistory.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ Releases follow the `major.minor.micro` scheme recommended by [PEP440](https://w
66
* `minor` increments add features but do not break API compatibility
77
* `micro` increments represent bugfix releases or improvements in documentation
88

9-
## Current development
9+
## 0.17.0
1010

1111
### API-breaking changes
1212

@@ -15,6 +15,8 @@ Releases follow the `major.minor.micro` scheme recommended by [PEP440](https://w
1515
### Bugfixes
1616

1717
### New features
18+
- [PR #2048](https://github.com/openforcefield/openff-toolkit/pull/2048): Adds NAGLChargesHandler. See [SMIRNOFF EP 11](https://github.com/openforcefield/standards/pull/71) for the new SMIRNOFF specification section and discussion.
19+
1820

1921
### Improved documentation and warnings
2022

‎docs/typing.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ During ``System`` creation, each ``ParameterHandler`` registered to a ``ForceFie
7575
ElectrostaticsHandler
7676
LibraryChargeHandler
7777
ToolkitAM1BCCHandler
78+
NAGLChargesHandler
7879
GBSAHandler
7980
ChargeIncrementModelHandler
8081
VirtualSiteHandler

‎openff/toolkit/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
GLOBAL_TOOLKIT_REGISTRY,
2222
AmberToolsToolkitWrapper,
2323
BuiltInToolkitWrapper,
24+
NAGLToolkitWrapper,
2425
OpenEyeToolkitWrapper,
2526
RDKitToolkitWrapper,
2627
ToolkitRegistry,
@@ -53,6 +54,7 @@
5354
"GLOBAL_TOOLKIT_REGISTRY": "openff.toolkit.utils.toolkits",
5455
"AmberToolsToolkitWrapper": "openff.toolkit.utils.toolkits",
5556
"BuiltInToolkitWrapper": "openff.toolkit.utils.toolkits",
57+
"NAGLToolkitWrapper": "openff.toolkit.utils.toolkits",
5658
"OpenEyeToolkitWrapper": "openff.toolkit.utils.toolkits",
5759
"RDKitToolkitWrapper": "openff.toolkit.utils.toolkits",
5860
"ToolkitRegistry": "openff.toolkit.utils.toolkits",

‎openff/toolkit/_tests/test_nagl.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import pytest
66
from openff.utilities import has_package, skip_if_missing
77

8+
from openff.nagl_models._dynamic_fetch import BadFileSuffixError
89
from openff.toolkit import Molecule, unit
910
from openff.toolkit._tests.create_molecules import (
1011
create_acetaldehyde,
@@ -14,8 +15,8 @@
1415
create_reversed_ethanol,
1516
)
1617
from openff.toolkit._tests.utils import requires_openeye
18+
from openff.toolkit.utils import GLOBAL_TOOLKIT_REGISTRY
1719
from openff.toolkit.utils.exceptions import (
18-
ChargeMethodUnavailableError,
1920
ToolkitUnavailableException,
2021
)
2122
from openff.toolkit.utils.nagl_wrapper import NAGLToolkitWrapper
@@ -38,6 +39,9 @@ def test_version(self):
3839

3940
assert parsed_version == NAGLToolkitWrapper()._toolkit_version
4041

42+
def test_nagl_in_global_toolkit_registry(self):
43+
assert NAGLToolkitWrapper in {type(tk) for tk in GLOBAL_TOOLKIT_REGISTRY.registered_toolkits}
44+
4145
@requires_openeye
4246
@pytest.mark.parametrize(
4347
"molecule_function",
@@ -123,8 +127,8 @@ def test_conformer_argument(self):
123127

124128
def test_unsupported_charge_method(self):
125129
with pytest.raises(
126-
ChargeMethodUnavailableError,
127-
match="Charge model hartree_fock not supported",
130+
BadFileSuffixError,
131+
match="Found an unrecognized file path extension on filename='hartree_fock'",
128132
):
129133
create_ethanol().assign_partial_charges(
130134
partial_charge_method="hartree_fock",

‎openff/toolkit/_tests/test_parameters.py

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
from openff.toolkit import ForceField, Molecule, Quantity, Topology, unit
1515
from openff.toolkit._tests.mocking import VirtualSiteMocking
1616
from openff.toolkit._tests.utils import does_not_raise
17+
from openff.toolkit.typing.engines.smirnoff import NAGLChargesHandler
1718
from openff.toolkit.typing.engines.smirnoff.parameters import (
1819
AngleHandler,
1920
BondHandler,
@@ -2722,6 +2723,201 @@ def test_charge_increment_one_ci_missing(self):
27222723
],
27232724
)
27242725

2726+
class TestNAGLChargesHandler:
2727+
def test_nagl_charges_handler_serialization(self):
2728+
handler = NAGLChargesHandler(model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt", skip_version_check=True)
2729+
assert handler.model_file == "openff-gnn-am1bcc-0.1.0-rc.3.pt"
2730+
handler_dict = handler.to_dict()
2731+
assert handler_dict["model_file"] == "openff-gnn-am1bcc-0.1.0-rc.3.pt"
2732+
2733+
def test_nagl_charges_handler_with_optional_fields(self):
2734+
# Test with model_file_hash
2735+
handler = NAGLChargesHandler(
2736+
model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
2737+
model_file_hash="144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0",
2738+
skip_version_check=True
2739+
)
2740+
assert handler.model_file == "openff-gnn-am1bcc-0.1.0-rc.3.pt"
2741+
assert handler.model_file_hash == "144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0"
2742+
assert handler.digital_object_identifier is None
2743+
2744+
# Test with digital_object_identifier
2745+
handler = NAGLChargesHandler(
2746+
model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
2747+
digital_object_identifier="10.5072/zenodo.203601",
2748+
skip_version_check=True
2749+
)
2750+
assert handler.model_file == "openff-gnn-am1bcc-0.1.0-rc.3.pt"
2751+
assert handler.model_file_hash is None
2752+
assert handler.digital_object_identifier == "10.5072/zenodo.203601"
2753+
2754+
# Test with both optional fields
2755+
handler = NAGLChargesHandler(
2756+
model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
2757+
model_file_hash="144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0",
2758+
digital_object_identifier="10.5072/zenodo.203601",
2759+
skip_version_check=True
2760+
)
2761+
assert handler.model_file == "openff-gnn-am1bcc-0.1.0-rc.3.pt"
2762+
assert handler.model_file_hash == "144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0"
2763+
assert handler.digital_object_identifier == "10.5072/zenodo.203601"
2764+
2765+
def test_nagl_charges_handler_serialization_with_optional_fields(self):
2766+
# Test serialization with all fields
2767+
handler = NAGLChargesHandler(
2768+
model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
2769+
model_file_hash="144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0",
2770+
digital_object_identifier="10.5072/zenodo.203601",
2771+
skip_version_check=True
2772+
)
2773+
handler_dict = handler.to_dict()
2774+
assert handler_dict["model_file"] == "openff-gnn-am1bcc-0.1.0-rc.3.pt"
2775+
assert handler_dict["model_file_hash"] == "144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0"
2776+
assert handler_dict["digital_object_identifier"] == "10.5072/zenodo.203601"
2777+
2778+
# Test deserialization via constructor
2779+
handler_from_dict = NAGLChargesHandler(**handler_dict)
2780+
assert handler_from_dict.model_file == "openff-gnn-am1bcc-0.1.0-rc.3.pt"
2781+
assert handler_from_dict.model_file_hash == "144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0"
2782+
assert handler_from_dict.digital_object_identifier == "10.5072/zenodo.203601"
2783+
2784+
def test_nagl_charges_handler_compatibility(self):
2785+
# Test compatible handlers (same model_file)
2786+
handler1 = NAGLChargesHandler(
2787+
model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
2788+
skip_version_check=True
2789+
)
2790+
handler2 = NAGLChargesHandler(
2791+
model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
2792+
model_file_hash="144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0",
2793+
skip_version_check=True
2794+
)
2795+
# Should not raise exception
2796+
handler1.check_handler_compatibility(handler2)
2797+
2798+
# Test incompatible handlers (different model_file)
2799+
handler3 = NAGLChargesHandler(
2800+
model_file="different-model-file.pt",
2801+
skip_version_check=True
2802+
)
2803+
with pytest.raises(IncompatibleParameterError, match="different model_files"):
2804+
handler1.check_handler_compatibility(handler3)
2805+
2806+
def test_nagl_charges_handler_defaults(self):
2807+
# Test that optional fields default to None
2808+
handler = NAGLChargesHandler(
2809+
model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
2810+
skip_version_check=True
2811+
)
2812+
assert handler.model_file_hash is None
2813+
assert handler.digital_object_identifier is None
2814+
2815+
def test_nagl_charges_handler_hash_compatibility(self):
2816+
"""Test compatibility checks for model_file_hash"""
2817+
# Test compatible handlers with same hash
2818+
handler1 = NAGLChargesHandler(
2819+
model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
2820+
model_file_hash="144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0",
2821+
skip_version_check=True
2822+
)
2823+
handler2 = NAGLChargesHandler(
2824+
model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
2825+
model_file_hash="144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0",
2826+
skip_version_check=True
2827+
)
2828+
# Should not raise exception
2829+
handler1.check_handler_compatibility(handler2)
2830+
2831+
# Test incompatible handlers with different hashes
2832+
handler3 = NAGLChargesHandler(
2833+
model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
2834+
model_file_hash="different_hash_value",
2835+
skip_version_check=True
2836+
)
2837+
with pytest.raises(IncompatibleParameterError, match="different model_file_hash values"):
2838+
handler1.check_handler_compatibility(handler3)
2839+
2840+
# Test compatibility when only one handler has hash (should be compatible)
2841+
handler4 = NAGLChargesHandler(
2842+
model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
2843+
skip_version_check=True
2844+
)
2845+
# Should not raise exception
2846+
handler1.check_handler_compatibility(handler4)
2847+
handler4.check_handler_compatibility(handler1)
2848+
2849+
def test_nagl_charges_handler_doi_compatibility(self):
2850+
"""Test compatibility checks for digital_object_identifier"""
2851+
# Test compatible handlers with same DOI
2852+
handler1 = NAGLChargesHandler(
2853+
model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
2854+
digital_object_identifier="10.5072/zenodo.203601",
2855+
skip_version_check=True
2856+
)
2857+
handler2 = NAGLChargesHandler(
2858+
model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
2859+
digital_object_identifier="10.5072/zenodo.203601",
2860+
skip_version_check=True
2861+
)
2862+
# Should not raise exception
2863+
handler1.check_handler_compatibility(handler2)
2864+
2865+
# Test incompatible handlers with different DOIs
2866+
handler3 = NAGLChargesHandler(
2867+
model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
2868+
digital_object_identifier="10.5072/zenodo.999999",
2869+
skip_version_check=True
2870+
)
2871+
with pytest.raises(IncompatibleParameterError, match="different digital_object_identifier values"):
2872+
handler1.check_handler_compatibility(handler3)
2873+
2874+
# Test compatibility when only one handler has DOI (should be compatible)
2875+
handler4 = NAGLChargesHandler(
2876+
model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
2877+
skip_version_check=True
2878+
)
2879+
# Should not raise exception
2880+
handler1.check_handler_compatibility(handler4)
2881+
handler4.check_handler_compatibility(handler1)
2882+
2883+
def test_nagl_charges_handler_combined_compatibility(self):
2884+
"""Test compatibility checks with both hash and DOI"""
2885+
# Test compatible handlers with same hash and DOI
2886+
handler1 = NAGLChargesHandler(
2887+
model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
2888+
model_file_hash="144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0",
2889+
digital_object_identifier="10.5072/zenodo.203601",
2890+
skip_version_check=True
2891+
)
2892+
handler2 = NAGLChargesHandler(
2893+
model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
2894+
model_file_hash="144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0",
2895+
digital_object_identifier="10.5072/zenodo.203601",
2896+
skip_version_check=True
2897+
)
2898+
# Should not raise exception
2899+
handler1.check_handler_compatibility(handler2)
2900+
2901+
# Test incompatible with same hash but different DOI
2902+
handler3 = NAGLChargesHandler(
2903+
model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
2904+
model_file_hash="144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0",
2905+
digital_object_identifier="10.5072/zenodo.999999",
2906+
skip_version_check=True
2907+
)
2908+
with pytest.raises(IncompatibleParameterError, match="different digital_object_identifier values"):
2909+
handler1.check_handler_compatibility(handler3)
2910+
2911+
# Test incompatible with different hash but same DOI
2912+
handler4 = NAGLChargesHandler(
2913+
model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
2914+
model_file_hash="different_hash_value",
2915+
digital_object_identifier="10.5072/zenodo.203601",
2916+
skip_version_check=True
2917+
)
2918+
with pytest.raises(IncompatibleParameterError, match="different model_file_hash values"):
2919+
handler1.check_handler_compatibility(handler4)
2920+
27252921

27262922
class TestGBSAHandler:
27272923
def test_create_default_gbsahandler(self):

‎openff/toolkit/typing/engines/smirnoff/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
IndexedParameterAttribute,
1919
LibraryChargeHandler,
2020
MappedParameterAttribute,
21+
NAGLChargesHandler,
2122
ParameterAttribute,
2223
ParameterHandler,
2324
ParameterList,

0 commit comments

Comments
 (0)