Implement NAGLChargesHandler

docs/releasehistory.md

@@ -6,7 +6,7 @@ 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
+## 0.17.0
 
 ### API-breaking changes
 
@@ -15,6 +15,8 @@ Releases follow the `major.minor.micro` scheme recommended by [PEP440](https://w
 ### Bugfixes
 
 ### New features
+- [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.
+
 
 ### Improved documentation and warnings
 

docs/typing.rst

@@ -75,6 +75,7 @@ During ``System`` creation, each ``ParameterHandler`` registered to a ``ForceFie
     ElectrostaticsHandler
     LibraryChargeHandler
     ToolkitAM1BCCHandler
+    NAGLChargesHandler
     GBSAHandler
     ChargeIncrementModelHandler
     VirtualSiteHandler

openff/toolkit/__init__.py

@@ -21,6 +21,7 @@
         GLOBAL_TOOLKIT_REGISTRY,
         AmberToolsToolkitWrapper,
         BuiltInToolkitWrapper,
+        NAGLToolkitWrapper,
         OpenEyeToolkitWrapper,
         RDKitToolkitWrapper,
         ToolkitRegistry,
@@ -53,6 +54,7 @@
     "GLOBAL_TOOLKIT_REGISTRY": "openff.toolkit.utils.toolkits",
     "AmberToolsToolkitWrapper": "openff.toolkit.utils.toolkits",
     "BuiltInToolkitWrapper": "openff.toolkit.utils.toolkits",
+    "NAGLToolkitWrapper": "openff.toolkit.utils.toolkits",
     "OpenEyeToolkitWrapper": "openff.toolkit.utils.toolkits",
     "RDKitToolkitWrapper": "openff.toolkit.utils.toolkits",
     "ToolkitRegistry": "openff.toolkit.utils.toolkits",

openff/toolkit/_tests/test_nagl.py

@@ -5,6 +5,7 @@
 import pytest
 from openff.utilities import has_package, skip_if_missing
 
+from openff.nagl_models._dynamic_fetch import BadFileSuffixError
 from openff.toolkit import Molecule, unit
 from openff.toolkit._tests.create_molecules import (
     create_acetaldehyde,
@@ -14,8 +15,8 @@
     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,
 )
 from openff.toolkit.utils.nagl_wrapper import NAGLToolkitWrapper
@@ -38,6 +39,9 @@ def test_version(self):
 
         assert parsed_version == NAGLToolkitWrapper()._toolkit_version
 
+    def test_nagl_in_global_toolkit_registry(self):
+        assert NAGLToolkitWrapper in {type(tk) for tk in GLOBAL_TOOLKIT_REGISTRY.registered_toolkits}
+
     @requires_openeye
     @pytest.mark.parametrize(
         "molecule_function",
@@ -123,8 +127,8 @@ def test_conformer_argument(self):
 
     def test_unsupported_charge_method(self):
         with pytest.raises(
-            ChargeMethodUnavailableError,
-            match="Charge model hartree_fock not supported",
+            BadFileSuffixError,
+            match="Found an unrecognized file path extension on filename='hartree_fock'",
         ):
             create_ethanol().assign_partial_charges(
                 partial_charge_method="hartree_fock",

openff/toolkit/_tests/test_parameters.py

@@ -14,6 +14,7 @@
 from openff.toolkit import ForceField, Molecule, Quantity, Topology, unit
 from openff.toolkit._tests.mocking import VirtualSiteMocking
 from openff.toolkit._tests.utils import does_not_raise
+from openff.toolkit.typing.engines.smirnoff import NAGLChargesHandler
 from openff.toolkit.typing.engines.smirnoff.parameters import (
     AngleHandler,
     BondHandler,
@@ -2722,6 +2723,201 @@ def test_charge_increment_one_ci_missing(self):
             ],
         )
 
+class TestNAGLChargesHandler:
+    def test_nagl_charges_handler_serialization(self):
+        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"
+
+    def test_nagl_charges_handler_with_optional_fields(self):
+        # Test with model_file_hash
+        handler = NAGLChargesHandler(
+            model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
+            model_file_hash="144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0",
+            skip_version_check=True
+        )
+        assert handler.model_file == "openff-gnn-am1bcc-0.1.0-rc.3.pt"
+        assert handler.model_file_hash == "144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0"
+        assert handler.digital_object_identifier is None
+
+        # Test with digital_object_identifier
+        handler = NAGLChargesHandler(
+            model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
+            digital_object_identifier="10.5072/zenodo.203601",
+            skip_version_check=True
+        )
+        assert handler.model_file == "openff-gnn-am1bcc-0.1.0-rc.3.pt"
+        assert handler.model_file_hash is None
+        assert handler.digital_object_identifier == "10.5072/zenodo.203601"
+
+        # Test with both optional fields
+        handler = NAGLChargesHandler(
+            model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
+            model_file_hash="144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0",
+            digital_object_identifier="10.5072/zenodo.203601",
+            skip_version_check=True
+        )
+        assert handler.model_file == "openff-gnn-am1bcc-0.1.0-rc.3.pt"
+        assert handler.model_file_hash == "144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0"
+        assert handler.digital_object_identifier == "10.5072/zenodo.203601"
+
+    def test_nagl_charges_handler_serialization_with_optional_fields(self):
+        # Test serialization with all fields
+        handler = NAGLChargesHandler(
+            model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
+            model_file_hash="144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0",
+            digital_object_identifier="10.5072/zenodo.203601",
+            skip_version_check=True
+        )
+        handler_dict = handler.to_dict()
+        assert handler_dict["model_file"] == "openff-gnn-am1bcc-0.1.0-rc.3.pt"
+        assert handler_dict["model_file_hash"] == "144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0"
+        assert handler_dict["digital_object_identifier"] == "10.5072/zenodo.203601"
+
+        # Test deserialization via constructor
+        handler_from_dict = NAGLChargesHandler(**handler_dict)
+        assert handler_from_dict.model_file == "openff-gnn-am1bcc-0.1.0-rc.3.pt"
+        assert handler_from_dict.model_file_hash == "144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0"
+        assert handler_from_dict.digital_object_identifier == "10.5072/zenodo.203601"
+
+    def test_nagl_charges_handler_compatibility(self):
+        # Test compatible handlers (same model_file)
+        handler1 = NAGLChargesHandler(
+            model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
+            skip_version_check=True
+        )
+        handler2 = NAGLChargesHandler(
+            model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
+            model_file_hash="144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0",
+            skip_version_check=True
+        )
+        # Should not raise exception
+        handler1.check_handler_compatibility(handler2)
+
+        # Test incompatible handlers (different model_file)
+        handler3 = NAGLChargesHandler(
+            model_file="different-model-file.pt",
+            skip_version_check=True
+        )
+        with pytest.raises(IncompatibleParameterError, match="different model_files"):
+            handler1.check_handler_compatibility(handler3)
+
+    def test_nagl_charges_handler_defaults(self):
+        # Test that optional fields default to None
+        handler = NAGLChargesHandler(
+            model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
+            skip_version_check=True
+        )
+        assert handler.model_file_hash is None
+        assert handler.digital_object_identifier is None
+
+    def test_nagl_charges_handler_hash_compatibility(self):
+        """Test compatibility checks for model_file_hash"""
+        # Test compatible handlers with same hash
+        handler1 = NAGLChargesHandler(
+            model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
+            model_file_hash="144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0",
+            skip_version_check=True
+        )
+        handler2 = NAGLChargesHandler(
+            model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
+            model_file_hash="144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0",
+            skip_version_check=True
+        )
+        # Should not raise exception
+        handler1.check_handler_compatibility(handler2)
+
+        # Test incompatible handlers with different hashes
+        handler3 = NAGLChargesHandler(
+            model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
+            model_file_hash="different_hash_value",
+            skip_version_check=True
+        )
+        with pytest.raises(IncompatibleParameterError, match="different model_file_hash values"):
+            handler1.check_handler_compatibility(handler3)
+
+        # Test compatibility when only one handler has hash (should be compatible)
+        handler4 = NAGLChargesHandler(
+            model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
+            skip_version_check=True
+        )
+        # Should not raise exception
+        handler1.check_handler_compatibility(handler4)
+        handler4.check_handler_compatibility(handler1)
+
+    def test_nagl_charges_handler_doi_compatibility(self):
+        """Test compatibility checks for digital_object_identifier"""
+        # Test compatible handlers with same DOI
+        handler1 = NAGLChargesHandler(
+            model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
+            digital_object_identifier="10.5072/zenodo.203601",
+            skip_version_check=True
+        )
+        handler2 = NAGLChargesHandler(
+            model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
+            digital_object_identifier="10.5072/zenodo.203601",
+            skip_version_check=True
+        )
+        # Should not raise exception
+        handler1.check_handler_compatibility(handler2)
+
+        # Test incompatible handlers with different DOIs
+        handler3 = NAGLChargesHandler(
+            model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
+            digital_object_identifier="10.5072/zenodo.999999",
+            skip_version_check=True
+        )
+        with pytest.raises(IncompatibleParameterError, match="different digital_object_identifier values"):
+            handler1.check_handler_compatibility(handler3)
+
+        # Test compatibility when only one handler has DOI (should be compatible)
+        handler4 = NAGLChargesHandler(
+            model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
+            skip_version_check=True
+        )
+        # Should not raise exception
+        handler1.check_handler_compatibility(handler4)
+        handler4.check_handler_compatibility(handler1)
+
+    def test_nagl_charges_handler_combined_compatibility(self):
+        """Test compatibility checks with both hash and DOI"""
+        # Test compatible handlers with same hash and DOI
+        handler1 = NAGLChargesHandler(
+            model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
+            model_file_hash="144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0",
+            digital_object_identifier="10.5072/zenodo.203601",
+            skip_version_check=True
+        )
+        handler2 = NAGLChargesHandler(
+            model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
+            model_file_hash="144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0",
+            digital_object_identifier="10.5072/zenodo.203601",
+            skip_version_check=True
+        )
+        # Should not raise exception
+        handler1.check_handler_compatibility(handler2)
+
+        # Test incompatible with same hash but different DOI
+        handler3 = NAGLChargesHandler(
+            model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
+            model_file_hash="144ed56e46c5b3ad80157b342c8c0f8f7340e4d382a678e30dd300c811646bd0",
+            digital_object_identifier="10.5072/zenodo.999999",
+            skip_version_check=True
+        )
+        with pytest.raises(IncompatibleParameterError, match="different digital_object_identifier values"):
+            handler1.check_handler_compatibility(handler3)
+
+        # Test incompatible with different hash but same DOI
+        handler4 = NAGLChargesHandler(
+            model_file="openff-gnn-am1bcc-0.1.0-rc.3.pt",
+            model_file_hash="different_hash_value",
+            digital_object_identifier="10.5072/zenodo.203601",
+            skip_version_check=True
+        )
+        with pytest.raises(IncompatibleParameterError, match="different model_file_hash values"):
+            handler1.check_handler_compatibility(handler4)
+
 
 class TestGBSAHandler:
     def test_create_default_gbsahandler(self):

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

@@ -18,6 +18,7 @@
     IndexedParameterAttribute,
     LibraryChargeHandler,
     MappedParameterAttribute,
+    NAGLChargesHandler,
     ParameterAttribute,
     ParameterHandler,
     ParameterList,