openforcefield / openff-evaluator

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

Improve pymbar compatibility #479

Closed mattwthompson closed 1 year ago

mattwthompson commented 1 year ago

Description

Provide a brief description of the PR's purpose here.

Todos

Notable points that this PR has either accomplished or will accomplish.

Questions

Status

codecov[bot] commented 1 year ago

Codecov Report

Merging #479 (a1ad71c) into main (a355cf3) will decrease coverage by 0.03%. The diff coverage is 66.66%.

Additional details and impacted files
mattwthompson commented 1 year ago

It'd take me a while to understand this code enough to write a test that captures all of these changes. I'm not sure how much this particular case is hit. This at least doesn't break anything we know of, and the changes at least look good in terms of the API changes. I can't find other uses in the codebase:

grep -r pymbar openff                                            12:56:16  ☁  more-pymbar ☀
openff/evaluator/tests/test_utils/test_timeseries.py:from openff.evaluator.utils.pymbar import detect_equilibration
openff/evaluator/tests/test_utils/test_timeseries.py:    """Compare the output of the ``analyze_time_series`` utility with ``pymbar``."""
openff/evaluator/utils/pymbar.py:Helper functions to simultaneously support pymbar 3 and 4.
openff/evaluator/utils/pymbar.py:    from pymbar import pymbar  # noqa
openff/evaluator/utils/pymbar.py:    import pymbar  # noqa
openff/evaluator/utils/pymbar.py:            from pymbar.timeseries import detectEquilibration as detect_equilibration
openff/evaluator/utils/pymbar.py:            from pymbar.timeseries import detect_equilibration
openff/evaluator/utils/timeseries.py:Based on the original implementation found in `pymbar
openff/evaluator/utils/timeseries.py:<https://github.com/choderalab/pymbar/tree/master/pymbar>`_
openff/evaluator/utils/timeseries.py:from pymbar.utils import ParameterError
openff/evaluator/utils/timeseries.py:    https://github.com/choderalab/pymbar. Here the code is extended support
openff/evaluator/utils/timeseries.py:    https://github.com/choderalab/pymbar. Here the code is extended support
openff/evaluator/utils/timeseries.py:            # Fix for issue https://github.com/choderalab/pymbar/issues/122
openff/evaluator/protocols/reweighting.py:import pymbar
openff/evaluator/protocols/reweighting.py:        mbar: pymbar.MBAR, target_reduced_potentials: ObservableArray
openff/evaluator/protocols/reweighting.py:        mbar = pymbar.MBAR(
openff/evaluator/protocols/reweighting.py:        mbar = pymbar.MBAR(
openff/evaluator/protocols/reweighting.py:        mbar: pymbar.MBAR,
openff/evaluator/protocols/reweighting.py:            # as pymbar does not expose an easier way to compute the average
openff/evaluator/protocols/reweighting.py:            from openff.evaluator.utils.pymbar import compute_expectations
openff/evaluator/protocols/reweighting.py:        mbar: pymbar.MBAR,

Personally, I'm fine with merging this as-is.

I'm not sure how to proceed here, any thoughts @jeff231li @j-wags?