-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle undetermined energies in BAR calculations #1098
Merged
Merged
Changes from all commits
Commits
Show all changes
42 commits
Select commit
Hold shift + click to select a range
5cc41b2
Simplify: avoid recomputing u_kln
mcwitt f4eb94d
Clean: use builtin alias for lru_cache(None)
mcwitt b651d53
Compute 2-state delta f using MBAR
mcwitt 7e8a9b2
Add failing test
mcwitt c51e808
Replace NaN with inf in u_kln
mcwitt 0b72da5
Clean: use works_from_ukln
mcwitt da1d363
Fix test
mcwitt 06066fb
Update test to use df_from_u_kln
mcwitt 352c27d
Add tests for uniform distributions with partial and zero overlap
mcwitt 7b9c853
Clean: use df_and_err_from_u_kln
mcwitt f3f2442
Strengthen tests: add comparison with exact dlogZ
mcwitt 135dad6
Clean: add file-level nogpu mark
mcwitt d3120b8
Strengthen partial overlap test to compare with exact result
mcwitt 0ed9719
Fix typo, add docstring note about u_kln convention
mcwitt 75da78a
Refactor to avoid forwarding kwargs
mcwitt 8da93ba
Fix missed update
mcwitt 4ab37d8
Fix sign errors
mcwitt 1c54793
Strengthen test: also check that error estimate is consistent
mcwitt 7495063
Add failing test
mcwitt 54e4b5c
Switch to -log(overlap) as cost function for bisection
mcwitt 7d41f55
Fix docstring
mcwitt 5f93cf6
Remove relative tolerance setting
mcwitt ec4cc2b
Clean, remove test case with zero overlap
mcwitt 35fe889
Merge branch 'master' into fix/handle-energy-overflow-bisection
mcwitt dc2c1b2
Add assertion for self-consistent iteration method
mcwitt 358edc3
Remove timeout logic from bootstrap_bar
mcwitt fd95420
Reduce number of bootstrap samples
mcwitt 11e30ff
Increase relative tolerance, reduce max iterations for MBAR
mcwitt be4d5c5
Use pymbar version released on pypi
mcwitt 92a9f70
Catch pymbar exception on incomplete convergence
mcwitt 4792f09
Update n_boostrap for consistency
mcwitt 2e0c9c1
Clean: axis=-1 -> axis=2
mcwitt f4a4b54
Fix and clean test
mcwitt f7fe229
Add assertions for pymbar behavior
mcwitt 0ddf9a0
Remove mentions of timeout in docstring
mcwitt f0f3414
Fix name of function in docstring, tweak formatting
mcwitt f7a1ab4
Add option to specify max solver iterations for bootstrapping
mcwitt 6950d82
Fix typo
mcwitt 28c6d9f
Remove obsolete assertion failure message
mcwitt e52fd30
Assert finite results with nan and inf inputs
mcwitt e01c2a3
Merge branch 'master' into fix/handle-energy-overflow-bisection
mcwitt 4a900c6
Merge branch 'master' into fix/handle-energy-overflow-bisection
mcwitt File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
numpy==1.23.5 | ||
scipy==1.10.1 | ||
pymbar==3.0.5 | ||
pymbar==3.1.0 | ||
networkx==2.8.8 | ||
matplotlib==3.7.1 | ||
rdkit==2022.3.5 | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
from unittest.mock import patch | ||
|
||
import numpy as np | ||
import pymbar | ||
import pytest | ||
from hypothesis import example, given, seed | ||
from hypothesis.strategies import integers | ||
|
@@ -11,13 +12,16 @@ | |
|
||
from timemachine.constants import DEFAULT_TEMP | ||
from timemachine.fe import free_energy, topology, utils | ||
from timemachine.fe.bar import ukln_to_ukn | ||
from timemachine.fe.free_energy import ( | ||
BarResult, | ||
HostConfig, | ||
IndeterminateEnergyWarning, | ||
MDParams, | ||
MinOverlapWarning, | ||
PairBarResult, | ||
batches, | ||
estimate_free_energy_bar, | ||
make_pair_bar_plots, | ||
run_sims_with_greedy_bisection, | ||
sample, | ||
|
@@ -351,3 +355,36 @@ def result_with_overlap(overlap): | |
] | ||
results, _, _ = run_sims_with_greedy_bisection_partial(min_overlap=0.4) | ||
assert len(results) == 1 + 2 | ||
|
||
|
||
def test_estimate_free_energy_bar_with_energy_overflow(): | ||
"""Ensure that we handle NaNs in u_kln inputs (e.g. due to overflow in potential evaluation).""" | ||
rng = np.random.default_rng(2023) | ||
u_kln = rng.uniform(-1, 1, (2, 2, 100)) | ||
|
||
_ = estimate_free_energy_bar(np.array([u_kln]), DEFAULT_TEMP) | ||
|
||
u_kln_with_nan = np.array(u_kln) | ||
u_kln_with_nan[0, 1, 10] = np.nan | ||
|
||
# pymbar.MBAR fails with LinAlgError | ||
with pytest.raises(SystemError, match="LinAlgError"): | ||
u_kn, N_k = ukln_to_ukn(u_kln_with_nan) | ||
_ = pymbar.MBAR(u_kn, N_k) | ||
|
||
# should return finite results with warning | ||
with pytest.warns(IndeterminateEnergyWarning, match="NaN"): | ||
result_with_nan = estimate_free_energy_bar(np.array([u_kln_with_nan]), DEFAULT_TEMP) | ||
|
||
assert np.isfinite(result_with_nan.dG) | ||
assert np.isfinite(result_with_nan.dG_err) | ||
|
||
u_kln_with_inf = np.array(u_kln) | ||
u_kln_with_inf[0, 1, 10] = np.inf | ||
|
||
# should give the same result with inf | ||
result_with_inf = estimate_free_energy_bar(np.array([u_kln_with_inf]), DEFAULT_TEMP) | ||
assert result_with_nan.dG == result_with_inf.dG | ||
assert result_with_nan.dG_err == result_with_inf.dG_err | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: assert finite There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added in e52fd30 |
||
np.testing.assert_array_equal(result_with_nan.dG_err_by_component, result_with_inf.dG_err_by_component) | ||
np.testing.assert_array_equal(result_with_nan.overlap, result_with_inf.overlap) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Should this be 3.1.0 or higher? Not sure why this is different from requirements
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<3.0.6
definitely won't work because of choderalab/pymbar#425, which was merged in3.0.6
. In general I prefer to use version constraints insetup.py
only to exclude known-incompatible versions.When upgrading, I opted to use the latest release <4.