Fix vsite fits#315
Conversation
Ohhhh, that's not good. |
|
It's now down to 1300 seconds, which is still quite long -- maybe this test should be skipped for speed reasons. |
| @@ -0,0 +1,291 @@ | |||
| import pytest | |||
|
|
|||
| has_openff_toolkit = True | |||
There was a problem hiding this comment.
This entire file just tests the new vsite parsing and assignment logic.
|
|
||
| class TestEvaluatorBromineStudy(ForceBalanceSystemTest): | ||
|
|
||
| class EvaluatorServerMixin: |
There was a problem hiding this comment.
Refactor the Evaluator tests to not repeat so much
| value_str, unit_str = quantity_str[:res.end()], quantity_str[res.end():] | ||
| # LPW 2023-01-23: Behavior of parameter unit string for "evaluated" parameter is undefined. | ||
| unit_str = "" | ||
| # unit_str = "" |
There was a problem hiding this comment.
Fixes parameter_eval
| vsprm.append(_openmm.OutOfPlaneSite_getWeight12(vs)) | ||
| vsprm.append(_openmm.OutOfPlaneSite_getWeight13(vs)) | ||
| vsprm.append(_openmm.OutOfPlaneSite_getWeightCross(vs)) | ||
| elif vstype == 'LocalCoordinatesSite': |
There was a problem hiding this comment.
OpenFF vsites are convereted to localcoordinatesites
|
|
||
| - pip: | ||
| # install branch of evaluator | ||
| - git+https://github.com/openforcefield/openff-evaluator.git@fix-vsite-fits |
There was a problem hiding this comment.
We will aim to get this merged and into a release ASAP, but this branch needed to be developed in tandem with the Evaluator one for testing to work (hence the same name)
| self._pending_estimate_request.results( | ||
| True, polling_interval=self._options.polling_interval | ||
| )[0] is None | ||
| self._pending_estimate_request.results(False)[0] is None |
There was a problem hiding this comment.
Having this blocking request here originally meant that other targets had to wait until Evaluator was done computing to even start -- which can be a while ...
| # Block until the Evaluator computation is finished. This is intentionally | ||
| # placed here (not in submit_jobs) so that Work Queue tasks submitted by other | ||
| # targets can run concurrently while we wait. | ||
| estimation_results, _ = self._pending_estimate_request.results( | ||
| True, polling_interval=self._options.polling_interval | ||
| ) | ||
| if estimation_results is None: | ||
| raise RuntimeError( | ||
| "No `EvaluatorServer` could be found to retrieve results from. " | ||
| "Please double check that a server is running, and that the connection " | ||
| "settings specified in the input script are correct." | ||
| ) | ||
|
|
There was a problem hiding this comment.
I moved (and added the same runtimeerror check) this from the submit_jobs function above.
2 participants
This PR enables ForceBalance to optimize virtual site (v-site) parameters in SMIRNOFF force fields. Previously, the code raised a hard NotImplementedError for any SMIRNOFF system with virtual sites. This PR removes that block and implements the full pipeline needed for v-site parameter fitting. As usual I've tried to add explanatory comments to code changes.
This is rebased off of #312 so the earlier commits aren't relevant -- as usual would recommend squash-merging over merging this though!
Biggest changes
New tests
I added two tests: