Update pymbar#317
Conversation
| # (water at 249.15 K/1 atm, 273.15 K/1 atm, 298.15 K/1 atm, 373.15 K/1 atm, | ||
| # 298.15 K/20 bar, 298.15 K/2000 bar; 801 snapshots each). | ||
| # Regenerate with: conda run -n <env> python tools/mbar_mre.py --save-ref | ||
| U_KLN_PATH = os.path.join(FIXTURE_DIR, 'U_kln.npy') |
There was a problem hiding this comment.
From here downwards: added tests comparing direct pymbar execution. The script referenced here is attached, not sure if I should add it to the repo or if it's clutter.
| _MBAR_SOLVER_KW = {} # v3: self-consistent-iteration default is fine | ||
| except ImportError: | ||
| import pymbar # pymbar 4: MBAR lives at pymbar top-level | ||
| _MBAR_SOLVER_KW = {'solver_protocol': 'robust'} # v4: default hybr diverges on some data |
There was a problem hiding this comment.
v3: used the adaptive solver by default. (https://pymbar.readthedocs.io/en/3.1.1/mbar.html#pymbar.MBAR)
v4: 'robust' uses adaptive, followed by L-BFGS (https://deepwiki.com/choderalab/pymbar/2.1-mbar-implementation) .
I'm not sure why, but plain swapping this out for the plain adaptive protocol results in tests failing, so maybe the fallback to L-BFGS is necessary. @mattwthompson I saw you managed to swap it out for adaptive in your PR, did you notice any differences?
There was a problem hiding this comment.
Unfortunately I don't recall, I was probably trying each option until one worked without really understanding the underlying behavior
lilyminium
force-pushed
the
update-pymbar
branch
from
bd10961 to
d7750f4
Compare
This reverts commit f50ba09.
|
Thanks so much for adding the pymbar tests. I agree this is ready to merge. |
3 participants
This PR adds a test to compare pymbar v3 and v4 compatiblity and also updates for v4. The main change seems to be moving from the default 'adaptive' solver in v3 to the 'robust' protocol in v4, which also calls the adaptive solver method.