Closed mattwthompson closed 1 year ago
I'm confused how this is happening; the code looks the same as ElectrostaticsHandler
but the behavior is different:
In [11]: ElectrostaticsHandler(version=0.3).version
Out[11]: <Version('0.4')>
In [12]: vdWHandler(version=0.3)
---------------------------------------------------------------------------
KeyError Traceback (most recent call last)
Cell In[12], line 1
----> 1 vdWHandler(version=0.3)
File ~/software/openff-toolkit/openff/toolkit/typing/engines/smirnoff/parameters.py:2852, in vdWHandler.__init__(self, **kwargs)
2843 logger.info(
2844 'Successfully up-converted vdW section from 0.3 to 0.4. `method="cutoff"` '
2845 'is now split into `periodic_method="cutoff"` '
2846 'and `nonperiodic_method="no-cutoff"`.'
2847 )
2849 else:
2850 raise NotImplementedError(
2851 "Failed to up-convert vdW section from 0.3 to 0.4. Did not know "
-> 2852 f'how to up-convert `method="{kwargs["method"]}"`.'
2853 )
2854 super().__init__(**kwargs)
KeyError: 'method'
I thought the issue was handling KeyError
by passing None
to dict.pop
, but that's done in both places.
My mistake, this should be fixed in #1689 by
diff --git a/openff/toolkit/typing/engines/smirnoff/parameters.py b/openff/toolkit/typing/engines/smirnoff/parameters.py
index acf2d936..290d1628 100644
--- a/openff/toolkit/typing/engines/smirnoff/parameters.py
+++ b/openff/toolkit/typing/engines/smirnoff/parameters.py
@@ -2834,7 +2834,7 @@ class vdWHandler(_NonbondedHandler):
logger.info("Attempting to up-convert vdW section from 0.3 to 0.4")
# This is the only supported value of "method" is version 0.3
- if kwargs.get("method") == "cutoff":
+ if kwargs.get("method") in ("cutoff", None):
kwargs["periodic_method"] = "cutoff"
kwargs["nonperiodic_method"] = "no-cutoff"
kwargs["version"] = 0.4
Describe the bug
The logic I added in
vdWHandler.__init__
(#1679, 0.14.2) assumed that themethod
kwarg was being passed, which usually was not the case. This results in a surprising error, surprising in the sense that I hadn't considered it. Passing themethod
kwarg through works fine.This naturally breaks some workflows using the API, which some packages like Evaluator relied on. Loading OFFXMLs seems fine.
To Reproduce