openforcefield / openff-evaluator

A physical property evaluation toolkit from the Open Forcefield Consortium.
https://docs.openforcefield.org/projects/evaluator
MIT License
54 stars 18 forks source link

Solvation free energy calculations failing #470

Closed mattwthompson closed 1 year ago

mattwthompson commented 2 years ago

Since sometime around the fall of 2021, the SolvationFreeEnergy property has been failing for reasons that are difficult to isolate. Something appears to be failing in PDB writing, likely not far away from openmmtools and/or Yank.

@lilyminium has a longer reproduction here, including more details that are not strictly necessary to copy here in a stub issue: https://openforcefieldgroup.slack.com/archives/C9NGV68BY/p1634576218009100

@jeff231li found this is still an issue when testing v0.4.0rc4 - he is now re-running tests a second time to ensure it's still an issue with OpenMM 7.7, which is believed to have fixed it.

mattwthompson commented 2 years ago

@jeff231li reports using MDTraj 1.9.4 fixes this

jeff231li commented 2 years ago

Yes, and later versions of MDTraj (>1.9.4) seem to fail.

mattwthompson commented 2 years ago

Wait, 1.9.4. is the only one that this works with? No older and no newer?

jeff231li commented 2 years ago

The versions that worked for me are 1.9.3 and 1.9.4. Later versions from 1.9.5 givers the error ValueError: cannot convert float NaN to integer.

mattwthompson commented 2 years ago

Hmm, I think we should continue with the release, but with the constraints to these versions. It'll probably cause some issues, but I'd rather fix those and get them in a patch release than delay the release altogether.

jeff231li commented 2 years ago

Agree 👍

mattwthompson commented 2 years ago

Re-opening as this is not completely resolved. It might end up being better in a new issue.

mattwthompson commented 1 year ago

@jeff231li Do you know if you were using OpenMM 7.7 in those failing tests? Are the tests something that you could tarball up and share? Free energy calculations aren't my wheelhouse.

jeff231li commented 1 year ago

@mattwthompson I re-tested the HFE calcs with OpenMM 7.7 and MDTraj 1.9.7 (which was released not too long ago). The calculations completed without the error we ran into previously ValueError: cannot convert float NaN to integer. However, I'm now getting a different error in the analysis part of the calculation. The error is related to switching to pymbar >=4 but openmmtools is still pinned to version 3. Here are the files I used to run the calculation including my conda environment HFE_calc.tar.gz.

mattwthompson commented 1 year ago

Thanks!

We could either patch openmmtools (https://github.com/choderalab/openmmtools/issues/629) or adapt the code to be compatible with both, like I saw Pascal do recently https://github.com/shirtsgroup/physical_validation/pull/220. I hope to be able to try one of them soon.