Implementation of Girsanov reweighting in deeptime MSM estimator #290

anyschaefer · 2024-04-30T07:53:30Z

We extended the MSM estimator class, such that the transition counts are reweighted according to the reweighting factor trajectory.

Joana-Lysiane Schäfer, Bettina G. Keller, Implementation of Girsanov reweighting in OpenMM and Deeptime

We demonstrated the correct functioning and error-free applicability of the newly implemented
functions using both a low-dimensional test system and a molecular system.
The corresponding reference "Implementation of Girsanov reweighting in OpenMM and Deeptime" is currently under review and will be publicly available shortly.
The required input files and setup for the evaluation of a reweighted Markov state model are available at reweightingtools

add GirsanovReweightingEstimator import

This module implements an estimator for Girsanov path reweighting for Markov state models.

add possibility to input list of g,M factor arrays

clonker · 2024-04-30T08:19:17Z

Super cool, thank you for your contribution! Do you think it is possible to add a sanity unit test to the new estimator? The failing tests are due to a tab/space mismatch. Let me know if you need support, I am happy to integrate this.

clonker · 2024-04-30T08:25:16Z

deeptime/markov/tools/estimation/api.py

@@ -213,7 +214,83 @@ def effective_count_matrix(dtrajs, lag, average='row', mact=1.0, n_jobs=None, ca
    return sparse.effective_counts.effective_count_matrix(dtrajs, lag, average=average, mact=mact, n_jobs=n_jobs,
                                                          callback=callback)

+def girsanov_reweighted_count_matrix(dtraj, lag, reweighting_factors,
+                                     sliding=True, sparse_return=True, nstates=None):
+    r"""Generate a Girsanov reweighted count matrix from given microstate trajectory. :footcite:`donati2017girsanov`


This will need an entry in references.bib to render correctly

we will add the reference as soon as it is published

… count mode other than sliding

codecov · 2024-05-06T07:48:34Z

Codecov Report

Attention: Patch coverage is 98.63014% with 1 line in your changes missing coverage. Please review.

Project coverage is 92.07%. Comparing base (f4ae6e4) to head (1da295d).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...ime/markov/tools/estimation/sparse/count_matrix.py	94.44%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #290      +/-   ##
==========================================
+ Coverage   92.03%   92.07%   +0.04%     
==========================================
  Files         142      143       +1     
  Lines       11657    11725      +68     
==========================================
+ Hits        10728    10796      +68     
  Misses        929      929

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

anyschaefer · 2024-05-06T07:55:37Z

Super cool, thank you for your contribution! Do you think it is possible to add a sanity unit test to the new estimator? The failing tests are due to a tab/space mismatch. Let me know if you need support, I am happy to integrate this.

Thank you, that's great! We added a sanity unit test for the new estimator and fixed the tap/space mismatch. When trying to run the tests with pytest tests/ or reinstall the modified package we get import errors due to different ..._bindings.cpython-311-x86_64-linux-gnu.so files. The problem concerns modules of deeptime that we have not modified, how to deal with this problem?

clonker · 2024-05-07T06:42:36Z

The bindings are part of the c++ module of deeptime, If all dependencies are available, you can set up a development environment with it using python setup.py develop. Otherwise you can just use pip install . and then use that module for testing. There are some tests failing right now in CI due to another part of deeptime, I have to fix that :) but generally, if your test runs through then that's fine for me.

clonker · 2024-05-07T08:19:12Z

If you update your branch against the current main, the errors unrelated to this PR should be gone. :)

clonker

The formatting is a bit off for some of the changes (like no spaces around equal signs) and you have a few problems in your doctests.

deeptime/markov/_girsanov_reweighted_transition_counting.py

deeptime/markov/tools/estimation/sparse/count_matrix.py

clonker · 2024-05-07T09:33:50Z

deeptime/markov/tools/estimation/sparse/count_matrix.py

+        factors = np.concatenate(factors)
+        data = factors
+    else:
+        raise NotImplementedError('An input format other than a tuple (g,M) for the reweighting factors is not implemented.')


could be covered by tests

deeptime/util/types.py

tests/markov/test_girsanov_reweighted_transition_counting.py

clonker · 2024-05-07T09:35:18Z

tests/markov/test_girsanov_reweighted_transition_counting.py

perhaps you can parametrize the tests to cover dense and sparse representations

anyschaefer · 2024-05-08T09:47:27Z

If you update your branch against the current main, the errors unrelated to this PR should be gone. :)

Unfortunately, I still have the same problem. Not only the tests in 'tests/' also the doctests gives me only message pointing to the import error.
My way to setup is: create anaconda environment with python 3.11; clone my updated fork or the original deeptime repository; in the deeptime folder with 'pip install .' install everything and with 'pytest tests/' or 'pytest --doctest-modules deeptime' I still get the import errors for the cpython extension files (e.g. 'deeptime.numeric._numeric_bindings') even if I can find them in the respective directories.
Do you know where the error can come from?

clonker · 2024-05-08T11:17:26Z

this is probably due to the way python discovers modules: first in the local directory. as there is a deeptime directory in your working dir, it'll find that one but without the compiled sources which are in your site packages :)

one way around this is using nox: pip install nox and then you just call nox inside the deeptime directory. that should take care of things. alternatively, just cd tests and then run

python -m pytest --doctest-modules --durations=20 --pyargs .

… added references

clonker · 2024-07-09T11:22:12Z

ah damn, i didn't implement numpy 2.0 support yet, hence the failures. i'll push a fix soon, then ci should be green (plusminus the test you're adding).

clonker · 2024-07-16T13:55:52Z

for all i am concerned this can be squashed and merged now. i still think the testing could be a bit more thorough (as indicated by coverage) but i'll leave that up to you if you want to provide it.

clonker · 2024-07-16T20:41:10Z

deeptime/markov/tools/estimation/api.py

+
+    >>> C_sliding.toarray()
+    array([[1., 2.],
+           [0., 1.]])


Sorry to bug about this again, but the test expectation does not match the result here, please do fix :)

Are you able to reproduce the test failure locally?

oh no, of course I'll fix it! unfortunately not until September.

clonker · 2024-10-22T13:03:40Z

Welcome back! It looks good to me now and the test failure is unrelated, so if you think it is ready, I will hit the merge button.

anyschaefer added 13 commits July 26, 2023 13:40

Update __init__.py

0a46560

add GirsanovReweightingEstimator import

Girsanov path reweighting estimator

06c93d7

This module implements an estimator for Girsanov path reweighting for Markov state models.

Update __init__.py

dd7ed1c

Update api.py

814c3ae

Update count_matrix.py

e09cc71

Update count_matrix.py

79b3d95

Update count_matrix.py

4fdd90a

Update _girsanov_reweighted_transition_counting.py

b95efdd

Update count_matrix.py

eefa782

Update count_matrix.py

473ec1b

add possibility to input list of g,M factor arrays

Update count_matrix.py

30d54b4

Update count_matrix.py

befc02e

Update AUTHORS

01066c0

clonker reviewed Apr 30, 2024

View reviewed changes

anyschaefer added 3 commits May 3, 2024 17:18

added a sanity unit test to new estimator

ea079dc

solved a tab/space mismatch

02e0033

Fixed input type of reweighting factors, inclusion of the error for a…

ed49320

… count mode other than sliding

anyschaefer mentioned this pull request May 7, 2024

Implementation of Girsanov reweighting factor reporter openmm/openmm#4533

Closed

clonker requested changes May 7, 2024

View reviewed changes

Merge branch 'deeptime-ml:main' into main

ffb4b4a

anyschaefer added 3 commits July 9, 2024 10:12

edit sparse and dense representation and add test

ddfff2e

changed input of reweighting factor tuple and added tests

7752925

added input tests for the Girsanov reweighted transition counting and…

648b36c

… added references

clonker approved these changes Jul 16, 2024

View reviewed changes

clonker requested changes Jul 16, 2024

View reviewed changes

anyschaefer and others added 2 commits October 22, 2024 10:26

Merge branch 'deeptime-ml:main' into main

85b7f25

corrected documentation

1da295d

clonker merged commit 5f85b3e into deeptime-ml:main Oct 23, 2024
13 of 15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementation of Girsanov reweighting in deeptime MSM estimator #290

Implementation of Girsanov reweighting in deeptime MSM estimator #290

anyschaefer commented Apr 30, 2024

clonker commented Apr 30, 2024

clonker Apr 30, 2024

anyschaefer May 6, 2024

codecov bot commented May 6, 2024 •

edited

Loading

anyschaefer commented May 6, 2024

clonker commented May 7, 2024

clonker commented May 7, 2024

clonker left a comment

clonker May 7, 2024

clonker May 7, 2024

anyschaefer commented May 8, 2024

clonker commented May 8, 2024 •

edited

Loading

clonker commented Jul 9, 2024

clonker commented Jul 16, 2024

clonker Jul 16, 2024

clonker Jul 16, 2024

anyschaefer Jul 17, 2024

clonker commented Oct 22, 2024

Implementation of Girsanov reweighting in deeptime MSM estimator #290

Implementation of Girsanov reweighting in deeptime MSM estimator #290

Conversation

anyschaefer commented Apr 30, 2024

clonker commented Apr 30, 2024

clonker Apr 30, 2024

Choose a reason for hiding this comment

anyschaefer May 6, 2024

Choose a reason for hiding this comment

codecov bot commented May 6, 2024 • edited Loading

Codecov Report

anyschaefer commented May 6, 2024

clonker commented May 7, 2024

clonker commented May 7, 2024

clonker left a comment

Choose a reason for hiding this comment

clonker May 7, 2024

Choose a reason for hiding this comment

clonker May 7, 2024

Choose a reason for hiding this comment

anyschaefer commented May 8, 2024

clonker commented May 8, 2024 • edited Loading

clonker commented Jul 9, 2024

clonker commented Jul 16, 2024

clonker Jul 16, 2024

Choose a reason for hiding this comment

clonker Jul 16, 2024

Choose a reason for hiding this comment

anyschaefer Jul 17, 2024

Choose a reason for hiding this comment

clonker commented Oct 22, 2024

codecov bot commented May 6, 2024 •

edited

Loading

clonker commented May 8, 2024 •

edited

Loading