Conversation
|
Will keep adding to this PR as needed, will also expand the news entry when the dust settles |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #951 +/- ##
==========================================
- Coverage 94.59% 91.64% -2.95%
==========================================
Files 134 134
Lines 9935 9935
==========================================
- Hits 9398 9105 -293
- Misses 537 830 +293
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
@mikemhenry do you want to add the allowable platforms list as a validator to the OpenmmEngineSettings class? I feel like we should document what we support somewhere, and it's probably as good a place to go "well you can do this but know that OpenCL and HIP aren't thoroughly validated. |
|
@IAlibay that is a good point! I will add that and also add a note somewhere in the docs, maybe we need a page that is like "Using OpenFE in Production" where we list what we have heavily validated and also list tips for using it, like generating all the partial charges before running transformations. |
| platform.setPropertyDefaultValue( | ||
| 'DeterministicForces', 'true') | ||
|
|
||
| if name != 'CUDA': |
There was a problem hiding this comment.
Do we need to set DeterministicForces or Percision for HIP?
There was a problem hiding this comment.
Good catch, I was able to do that with the OpenMM API, but I need to test on a real GPU to see if it works (as in, setting those properties worked on my openMM HIP build but it could crash as soon as I try and run a MD step on the GPU)
There was a problem hiding this comment.
Is there an easy way to test this out? I.e. would spinning up an AWS runner that uses an AMD card work?
|
So we don't currently have any validators: A literal would be my preference -- users can deal with errors of using |
I personally would prefer it if it was a validator - mainly you get things like "OpenCL" | "opencl". |
Will use this PR to track what we need to support HIP. Related PRs and Issues:
#826
choderalab/openmmtools#753
Checklist
newsentryDevelopers certificate of origin