From 3580cbb6fab52a22964161e10a61f7092df02b8d Mon Sep 17 00:00:00 2001 From: Jeffrey Wagner Date: Mon, 23 Jun 2025 11:27:50 -0700 Subject: [PATCH 1/3] add parameterhandler.hash and tests --- openff/toolkit/_tests/test_parameters.py | 38 +++++++++++++++++++ .../typing/engines/smirnoff/parameters.py | 12 ++++++ 2 files changed, 50 insertions(+) diff --git a/openff/toolkit/_tests/test_parameters.py b/openff/toolkit/_tests/test_parameters.py index bd9ae0836..e89f56e8d 100644 --- a/openff/toolkit/_tests/test_parameters.py +++ b/openff/toolkit/_tests/test_parameters.py @@ -822,6 +822,44 @@ class MyParameterHandler(ParameterHandler): ): handler.create_force() + def test_hash(self): + bh = BondHandler(skip_version_check=True, allow_cosmetic_attributes=True) + + hash1 = hash(bh) + # Ensure hash changes when a new parameter is added + bh.add_parameter( + { + "smirks": "[*:1]-[*:2]", + "length": 1 * unit.angstrom, + "k": 10 * unit.kilocalorie / unit.mole / unit.angstrom ** 2, + "id": "b0", + } + ) + hash2 = hash(bh) + assert hash1 != hash2 + + # Ensure hash changes when another parameter that differs only by SMIRKS is added + bh.add_parameter( + { + "smirks": "[C:1]-[C:2]", + "length": 1 * unit.angstrom, + "k": 10 * unit.kilocalorie / unit.mole / unit.angstrom ** 2, + "id": "b0", + } + ) + hash3 = hash(bh) + assert hash2 != hash3 + + # Ensure hash changes when a cosmetic attribute is added + bh.parameters[0].add_cosmetic_attribute("foo", "bar") + hash4 = hash(bh) + assert hash3 != hash4 + + # Ensure hash changes when parameters are reordered + param = bh.parameters.pop(0) + bh.parameters.append(param) + hash5 = hash(bh) + assert hash4 != hash5 class TestParameterList: """Test capabilities of ParameterList for accessing and manipulating SMIRNOFF parameter definitions.""" diff --git a/openff/toolkit/typing/engines/smirnoff/parameters.py b/openff/toolkit/typing/engines/smirnoff/parameters.py index b42fcba8b..118237cdd 100644 --- a/openff/toolkit/typing/engines/smirnoff/parameters.py +++ b/openff/toolkit/typing/engines/smirnoff/parameters.py @@ -54,6 +54,7 @@ import copy import functools +import hashlib import inspect import logging import re @@ -1905,6 +1906,17 @@ def __init__( # Initialize ParameterAttributes and cosmetic attributes. super().__init__(allow_cosmetic_attributes=allow_cosmetic_attributes, **kwargs) + def __hash__(self): + """ + Hash a ParameterHandler and all of its contents (INCLUDING cosmetic attributes). + + This method does not attempt to return the same hash for ParameterHandlers with equivalent + physics/chemistry but different cosmetic attributes. Instead this is a hash of all of the + ParameterHandler's contents, even if they don't affect system creation in any way. + """ + # It's somewhat silly to stringify a dict but if it works, it works + return hash(str(self.to_dict())) + def _add_parameters(self, section_dict, allow_cosmetic_attributes=False): """ Extend the ParameterList in this ParameterHandler using a SMIRNOFF data source. From 077dbf6de5099bcb5c46fd59cde27af6f40a9090 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 23 Jun 2025 18:34:31 +0000 Subject: [PATCH 2/3] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- openff/toolkit/typing/engines/smirnoff/parameters.py | 1 - 1 file changed, 1 deletion(-) diff --git a/openff/toolkit/typing/engines/smirnoff/parameters.py b/openff/toolkit/typing/engines/smirnoff/parameters.py index 118237cdd..0ac29e922 100644 --- a/openff/toolkit/typing/engines/smirnoff/parameters.py +++ b/openff/toolkit/typing/engines/smirnoff/parameters.py @@ -54,7 +54,6 @@ import copy import functools -import hashlib import inspect import logging import re From 7ff3cc95db54ffe3ae89f0e897b7df74556d481b Mon Sep 17 00:00:00 2001 From: Jeffrey Wagner Date: Wed, 25 Jun 2025 10:13:32 -0700 Subject: [PATCH 3/3] checkpoint progress on trying to avoid inefficiently stringifying quantities --- openff/toolkit/_tests/test_parameters.py | 11 ++++++- .../typing/engines/smirnoff/parameters.py | 31 ++++++++++++++++--- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/openff/toolkit/_tests/test_parameters.py b/openff/toolkit/_tests/test_parameters.py index e89f56e8d..683cc30bd 100644 --- a/openff/toolkit/_tests/test_parameters.py +++ b/openff/toolkit/_tests/test_parameters.py @@ -850,10 +850,14 @@ def test_hash(self): hash3 = hash(bh) assert hash2 != hash3 + bh.add_cosmetic_attribute("fizz", "buzz") + hash3p5 = hash(bh) + assert hash3 != hash3p5 + # Ensure hash changes when a cosmetic attribute is added bh.parameters[0].add_cosmetic_attribute("foo", "bar") hash4 = hash(bh) - assert hash3 != hash4 + assert hash3p5 != hash4 # Ensure hash changes when parameters are reordered param = bh.parameters.pop(0) @@ -861,6 +865,11 @@ def test_hash(self): hash5 = hash(bh) assert hash4 != hash5 + # Ensure hash doesn't change when the contents haven't changed + hash6 = hash(bh) + assert hash5 == hash6 + + class TestParameterList: """Test capabilities of ParameterList for accessing and manipulating SMIRNOFF parameter definitions.""" diff --git a/openff/toolkit/typing/engines/smirnoff/parameters.py b/openff/toolkit/typing/engines/smirnoff/parameters.py index 0ac29e922..d405c6e30 100644 --- a/openff/toolkit/typing/engines/smirnoff/parameters.py +++ b/openff/toolkit/typing/engines/smirnoff/parameters.py @@ -1910,11 +1910,32 @@ def __hash__(self): Hash a ParameterHandler and all of its contents (INCLUDING cosmetic attributes). This method does not attempt to return the same hash for ParameterHandlers with equivalent - physics/chemistry but different cosmetic attributes. Instead this is a hash of all of the - ParameterHandler's contents, even if they don't affect system creation in any way. - """ - # It's somewhat silly to stringify a dict but if it works, it works - return hash(str(self.to_dict())) + physics/chemistry but different cosmetic attributes or units. Instead this is a hash of all + of the ParameterHandler's contents, even if they don't affect system creation in any way. + """ + handler_string = '' + attribute_dict = self.__dict__ + for key, val in attribute_dict.items(): + if isinstance(val, ParameterList): + handler_string += f'___{key}' + for parameter in val: + # print(parameter) + for attribute_name, attribute_val in parameter.__dict__.items(): + # print(attribute_name, attribute_val) + if isinstance(attribute_val, list): + #print(attribute_val) + if len(attribute_val) > 0 and isinstance(attribute_val[0], Quantity): + attribute_val = tuple([(i.m, hash(i.units)) for i in attribute_val]) + #else: + if isinstance(attribute_val, Quantity): + # print(dir(attribute_val)) + attribute_val = (attribute_val.m, hash(attribute_val.units)) + # break + handler_string += f'__{attribute_name}_{attribute_val}' + else: + handler_string += f'{key}__{val}' + + return hash(handler_string) def _add_parameters(self, section_dict, allow_cosmetic_attributes=False): """