Not being able to load the 2.1.0 forcefield file #1635
Comments
ijpulidos
changed the title
|
Just in case it helps, loading the previous |
|
It was loading fine for me on Linux with toolkit version |
|
@ijpulidos might be related to #1632 but the toolkit packages were upgraded so it shouldn't pull 0.22 now. |
|
@ijpulidos Could you try again after downgrading
This is new to me. I'd initially thought this was related to #1632 but maybe it's something new. |
|
I'm able to reproduce this error with OFFTK 0.13.1 and pint 0.22 running |
|
I can't reproduce this now that the one-liner pulls down Pint 0.21. Forcing Pint 0.22 and applying the below patch shows that it's choking on trying to convert the diff --git a/openff/toolkit/utils/utils.py b/openff/toolkit/utils/utils.py
index c3542c8e..36837b93 100644
--- a/openff/toolkit/utils/utils.py
+++ b/openff/toolkit/utils/utils.py
@@ -198,6 +198,9 @@ def string_to_quantity(quantity_string) -> Union[str, int, float, unit.Quantity]
quantity = unit.Quantity(quantity_string)
except (TokenError, UndefinedUnitError):
return quantity_string
+ except AssertionError:
+ print(quantity_string)
+ return quantity_string
# TODO: Should intentionally unitless array-likes be Quantity objects
# or their raw representation?$ python -c "from openff.toolkit import ForceField; ForceField('openff-2.1.0.offxml')"
Li+
Na+
K+
Rb+
Cs+
F-
Cl-
Br-
I-This is reminiscent of #1633 though the behavior is different; parsing something into an object when it should remain a string is different than throwing an exception. Here is a more direct reproduction of the fundamental issue: from pint import __version__, UnitRegistry
print(__version__)
# 0.22
unit = UnitRegistry()
for ion in [
"Li+",
"Na+",
"K+",
"Rb+",
"Cs+",
"F-",
"Cl-",
"Br-",
"I-",
]:
try:
unit.Quantity(ion)
except AssertionError:
pass
# raises no other exception#1640 fixes this, but more robust parsing logic than blindly chucking strings to |
|
Oh, and for why 2.0.0 was loadable and 2.1.0 was not: |
|
This should be resolved via #1640 in the next release (probably 0.13.2, no planned date). The fix was somewhat superficial and a deeper fix may happen in the future. But for now, it should work okay. I'd like to explicitly thank @ijpulidos for providing such a thorough bug report. For mysterious behaviors that seems like regressions, the more tea leaves to read the better, and this one came with tons of useful information. Thanks! |
I tried to track this down and I think the change happened when I got a FF with delocalized smirks patterns from @chapincavender , I don't know if this is a change in toolkit itself or a custom function to write library charges. Any idea if this would be an issue for other downstream users who parse our FF directly without toolkit? |
|
It shouldn't be, but there's really no way to control others' solutions. I posted that only to help diagnose the issue - there was no obvious reason why 2.0 and 2.1 would be parsed differently. I'm not advocating for that to be changed. |
|
This is a change I made to the LibraryCharge section when I added the library charges for amino acids, so it's not a change in toolkit parsing. The entries for TIP3P library charges used |
|
I think you made the right call there, Chapin. I've looked at the openff-2.0.0 offxmls dozens of times and never noticed the inconsistency in use of LibraryCharge id/name. I agree with the decision to put all the identifiers into the |
Describe the bug
I noticed that the latest release (0.13.1) is having errors with loading the
2.1.0forcefield release. The same happens with2.1.0.rc-1.Even though the files exist in the directory
To Reproduce
Output
Computing environment (please complete the following information):
mamba create -n openff-ffs-test -c conda-forge openff-forcefields openmmforcefieldsconda listconda list
Additional context
The text was updated successfully, but these errors were encountered: