Skip to content

Commit

Permalink
Merge branch 'main' into fix/still-some-flaky-tests
Browse files Browse the repository at this point in the history
  • Loading branch information
mikemhenry authored Oct 3, 2024
2 parents 7a1a7d9 + 9334bc9 commit 6b56df1
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 2 deletions.
1 change: 1 addition & 0 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ jobs:
- name: Setup micromamba
uses: mamba-org/setup-micromamba@v1
with:
micromamba-version: '2.0.0-0'
environment-file: devtools/conda-envs/test_env.yaml
environment-name: openmmtools-test
create-args: >-
Expand Down
7 changes: 7 additions & 0 deletions openmmtools/multistate/multistateanalyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import copy
import inspect
import logging
from packaging.version import Version
import re
from typing import Optional, NamedTuple, Union

Expand All @@ -37,6 +38,7 @@
import simtk.unit as units
from scipy.special import logsumexp

from openmmtools.multistate import pymbar
from openmmtools import multistate, utils, forces
from openmmtools.multistate.pymbar import (
statistical_inefficiency_multiple,
Expand Down Expand Up @@ -567,6 +569,11 @@ def __init__(self, reporter, name=None, reference_states=(0, -1),
self.reference_states = reference_states
self._user_extra_analysis_kwargs = analysis_kwargs # Store the user-specified (higher priority) keywords

# If we are using pymbar 4, change the default behavior to use the robust solver protocol if the user
# didn't set a kwarg to control the solver protocol
if Version(pymbar.__version__) >= Version("4") and "solver_protocol" not in self._user_extra_analysis_kwargs:
self._user_extra_analysis_kwargs["solver_protocol"] = "robust"

# Initialize cached values that are read or derived from the Reporter.
self._cache = {} # This cache should be always set with _update_cache().
self.clear()
Expand Down
3 changes: 2 additions & 1 deletion openmmtools/multistate/pymbar.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
subsample_correlated_data,
statistical_inefficiency
)
from pymbar import MBAR
from pymbar import MBAR, __version__
from pymbar.utils import ParameterError
except ImportError:
# pymbar < 4
Expand All @@ -22,6 +22,7 @@
)
from pymbar import MBAR
from pymbar.utils import ParameterError
from pymbar.version import short_version as __version__


def _pymbar_bar(
Expand Down
62 changes: 61 additions & 1 deletion openmmtools/tests/test_sampling.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@
import shutil
import sys
import tempfile
import time
from io import StringIO

import numpy as np
import yaml

import pytest
import requests

try:
import openmm
Expand Down Expand Up @@ -1863,7 +1865,7 @@ def test_analysis_opens_without_checkpoint(self):
del reporter
self.REPORTER(storage_path, checkpoint_storage=cp_file_mod, open_mode="r")

@pytest.mark.flaky(reruns=3)
@pytest.mark.skipif(sys.platform == "darwin", reason="seg faults on osx sometimes")
def test_storage_reporter_and_string(self):
"""Test that creating a MultiState by storage string and reporter is the same"""
thermodynamic_states, sampler_states, unsampled_states = copy.deepcopy(
Expand Down Expand Up @@ -2614,6 +2616,64 @@ def test_resume_velocities_from_legacy_storage(self):
state.velocities.value_in_unit_system(unit.md_unit_system) != 0
), "At least some velocity in sampler state from new checkpoint is expected to different from zero."

@pytest.fixture
def download_nc_file(tmpdir):
# See https://github.com/choderalab/pymbar/issues/419#issuecomment-1718386779
# and https://github.com/choderalab/openmmtools/pull/735#issuecomment-2378070388
# if this file ever starts to 404
FILE_URL = "https://github.com/user-attachments/files/17156868/ala-thr.zip"
MAX_RETRIES = 3
RETRY_DELAY = 2 # Delay between retries (in seconds)
file_name = os.path.join(tmpdir, "ala-thr.nc")
retries = 0
while retries < MAX_RETRIES:
try:
# Send GET request to download the file
response = requests.get(FILE_URL, timeout=20) # Timeout to avoid hanging
response.raise_for_status() # Raise HTTPError for bad responses (4xx/5xx)
with open(file_name, "wb") as f:
f.write(response.content)
# File downloaded successfully, break out of retry loop
break

except (requests.exceptions.RequestException, requests.exceptions.HTTPError) as e:
retries += 1
if retries >= MAX_RETRIES:
pytest.fail(f"Failed to download file after {MAX_RETRIES} retries: {e}")
else:
print(f"Retrying download... ({retries}/{MAX_RETRIES})")
time.sleep(RETRY_DELAY) # Wait before retrying
yield file_name


def test_pymbar_issue_419(download_nc_file):
"""
This test checks that a nc file from a ala-thr mutation simulation converges.
With pymbar 4 default (as of 2024-10-02) solver fails to converge.
With pymbar 3 defaults, the solver does converge.
With PR #735 (https://github.com/choderalab/openmmtools/pull/735) we updated
the MultiStateSamplerAnalyzer to use the "robust" sampler when using pymbar4.
See https://github.com/choderalab/pymbar/issues/419#issuecomment-1718386779 for more
information on how the file was generated.
"""


from openmmtools.multistate import MultiStateReporter, MultiStateSamplerAnalyzer

n_iterations = 1000
reporter_file = download_nc_file
reporter = MultiStateReporter(reporter_file)
analyzer = MultiStateSamplerAnalyzer(reporter, max_n_iterations=n_iterations)
f_ij, df_ij = analyzer.get_free_energy()
# free energy
assert f_ij[0, -1] == pytest.approx(-52.00083148433459)
# error
assert df_ij[0, -1] == pytest.approx(0.21365627649558516)


# ==============================================================================
# MAIN AND TESTS
Expand Down

0 comments on commit 6b56df1

Please sign in to comment.