Update smirnoff_hack.py for OpenFF Toolkit 0.10.3#248
Update smirnoff_hack.py for OpenFF Toolkit 0.10.3#248mattwthompson wants to merge 3 commits intoleeping:masterfrom
Conversation
|
CI seems to be failing for other reasons (pip and/or other deps changing). I might be able to have a look later. |
j-wags
left a comment
There was a problem hiding this comment.
Ahh gosh, sorry, I keep forgetting about this file. Yes, the OFF Toolkit 0.10.3 release yesterday added a kwarg to the generate_conformers method signature which broke the caching logic here. I'm sorry for the inconvenience @leeping.
This code change looks correct to me. @leeping - ForceBalance's CI hasn't run since the 0.10.3 OFF toolkit release, so I can't show you the problem, but if we run CI on master right now it should fail because of this problem.
Apologies again, I think our ask is both for this PR and for a ForceBalance release. There may be other issues happening too with other deps and we could discuss how to handle those as well.
625df4d to
7bc9350
Compare
|
I've updated this to be self-contained to the changes associated with the toolkit. I opened #249 in an effort to fix other failing tests. Hopefully that can be accomplished before this one. |
|
The approach from #231 (switching from explicit listing of args and kwargs to using In the long run it'd be great to fully move the caching out of forcebalance and into the toolkit. Maybe we could coordinate with @leeping on that so we can synchronize making that change and a subsequent release in the future? Then we could stabilize the FB interface and not cause this kind of problem in the future. |
|
Agreed, I've opened #251 with a smaller set of changes |
We released a new version of the toolkit yesterday that broke
smirnoff_hackby nature of adding a named argument togenerate_conformers- fixing it should be a matter of string the new argument through the appropriate functions.