mbar overlap matrix plot #107

xiki-tempula · 2020-08-24T15:38:32Z

I have ported the MBAR overlap matrix to this library and have written some documentation.
I don't know how to write tests for the plot so I have just written a script where the plot runs.

Related to #73

codecov · 2020-08-24T16:03:25Z

Codecov Report

Merging #107 into master will increase coverage by 0.04%.
The diff coverage is 97.91%.

@@            Coverage Diff             @@
##           master     #107      +/-   ##
==========================================
+ Coverage   97.26%   97.30%   +0.04%     
==========================================
  Files          12       14       +2     
  Lines         695      743      +48     
  Branches      141      150       +9     
==========================================
+ Hits          676      723      +47     
  Misses          5        5              
- Partials       14       15       +1

Impacted Files	Coverage Δ
src/alchemlyb/visualisation/mbar_matrix.py	`97.77% <97.77%> (ø)`
src/alchemlyb/estimators/mbar_.py	`96.15% <100.00%> (+0.32%)`	⬆️
src/alchemlyb/visualisation/__init__.py	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7fcdf7a...8cfd520. Read the comment docs.

orbeckst

Thank you for the contribution. I have some comments inline. Could you please also fix the spelling of "matrix" throughout?

Finally, could you include an image of the plot in the docs and attach the equivalent image from alchemical-analysis.py for the same data so that we can see in how far it reproduces the old code? Thanks!

orbeckst · 2020-08-25T08:19:33Z

src/alchemlyb/visualisation/mbar_martix.py

+"""
+from __future__ import division
+import matplotlib
+matplotlib.use('Agg')


remove, this is up to the user

@orbeckst Thank you for the review. I got a problem with CI for py2, where if I don't add this line in. I would get

src/alchemlyb/tests/test_visualisation.py:15: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ src/alchemlyb/visualisation/mbar_martix.py:31: in plot_mbar_omatrix fig, ax = plt.subplots(figsize=(size / 2, size / 2)) ../../../virtualenv/python2.7.15/lib/python2.7/site-packages/matplotlib/pyplot.py:1184: in subplots fig = figure(**fig_kw) ../../../virtualenv/python2.7.15/lib/python2.7/site-packages/matplotlib/pyplot.py:533: in figure **kwargs) ../../../virtualenv/python2.7.15/lib/python2.7/site-packages/matplotlib/backend_bases.py:161: in new_figure_manager return cls.new_figure_manager_given_figure(num, fig) ../../../virtualenv/python2.7.15/lib/python2.7/site-packages/matplotlib/backends/_backend_tk.py:1046: in new_figure_manager_given_figure window = Tk.Tk(className="matplotlib")

As is seen https://travis-ci.org/github/alchemistry/alchemlyb/jobs/720700286, Google suggests that this is one way of solving the problem. I'm not familiar with CI so I'm not sure of what is the best way to process.

@orbeckst I'm sorry for the confusion. I originally put the line matplotlib.use('Agg') in order to avoid the travis failing for py2. I wonder what is your suggestion?

Sorry to be brief: see https://github.com/MDAnalysis/mdanalysis/pull/2798/files#diff-354f30a63fb0907d4ad57269548329e3R39 : add the env var MPLBACKEND: "agg" to the CI.

orbeckst · 2020-08-25T08:21:20Z

src/alchemlyb/visualisation/mbar_martix.py

+matplotlib.use('Agg')
+import matplotlib.pyplot as plt
+import numpy as np
+def plot_mbar_omatrix(matrix, skip_lambda_index=[], ax=None):


write out omatrix => overlap_matrix: just be explicit

orbeckst · 2020-08-25T08:22:11Z

src/alchemlyb/tests/test_visualisation.py

+    overlap_maxtrix = mbar_coul.overlap_maxtrix
+    overlap_maxtrix[0,0] = 0.0025
+    overlap_maxtrix[-1, -1] = 0.9975
+    plot_mbar_omatrix(overlap_maxtrix)


assert that you return an axes object

orbeckst · 2020-08-25T08:22:45Z

src/alchemlyb/tests/test_visualisation.py

+
+    from alchemlyb.visualisation.mbar_martix import plot_mbar_omatrix
+    plot_mbar_omatrix(mbar_coul.overlap_maxtrix)
+    plot_mbar_omatrix(mbar_coul.overlap_maxtrix, [1,])


assert isinstance axes

src/alchemlyb/tests/test_visualisation.py

orbeckst · 2020-08-25T08:23:20Z

src/alchemlyb/tests/test_visualisation.py

+
+def test_plot_mbar_omatrix():
+    '''Just test if the plot runs'''
+    import pandas as pd


imports at top of test file

orbeckst · 2020-08-25T08:25:47Z

src/alchemlyb/estimators/mbar_.py

@@ -85,6 +88,7 @@ def fit(self, u_nk):
                           verbose=self.verbose)

        self.states_ = u_nk.columns.values.tolist()
+        self.overlap_maxtrix = self._mbar.computeOverlap()['matrix']


Make it a managed attribute so that it is not computed by default or make it a method compute_overlap_matrix() -- not sure if @dotsdl has a preference.

maxtrix => matrix

orbeckst · 2020-08-25T08:27:34Z

docs/visualisation/alchemlyb.visualisation.plot_mbar_omatrix.rst

@@ -0,0 +1,16 @@
+Plot Overlap Matrix from MBar


docs/visualisation.rst

orbeckst · 2020-08-25T15:47:05Z

In MDAnalysis we solved this by setting the backend environment variable to agg; it’s either already in develop and/or in the 1.0.1 backport branch,IIRC.

…

Am 25.08.2020 um 04:57 schrieb Zhiyi Wu ***@***.***>: @xiki-tempula commented on this pull request. In src/alchemlyb/visualisation/mbar_martix.py: > @@ -0,0 +1,81 @@ +"""Functions for Plotting the overlay matrix for the Mbar estimator. + +""" +from __future__ import division +import matplotlib +matplotlib.use('Agg') @orbeckst Thank you for the review. I got a problem with CI for py2, where if I don't add this line in. I would get src/alchemlyb/tests/test_visualisation.py:15: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ src/alchemlyb/visualisation/mbar_martix.py:31: in plot_mbar_omatrix fig, ax = plt.subplots(figsize=(size / 2, size / 2)) ../../../virtualenv/python2.7.15/lib/python2.7/site-packages/matplotlib/pyplot.py:1184: in subplots fig = figure(**fig_kw) ../../../virtualenv/python2.7.15/lib/python2.7/site-packages/matplotlib/pyplot.py:533: in figure **kwargs) ../../../virtualenv/python2.7.15/lib/python2.7/site-packages/matplotlib/backend_bases.py:161: in new_figure_manager return cls.new_figure_manager_given_figure(num, fig) ../../../virtualenv/python2.7.15/lib/python2.7/site-packages/matplotlib/backends/_backend_tk.py:1046: in new_figure_manager_given_figure window = Tk.Tk(className="matplotlib") As is seen https://travis-ci.org/github/alchemistry/alchemlyb/jobs/720700286, Google suggests that this is one way of solving the problem. I'm not familiar with CI so I'm not sure of what is the best way to process. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

orbeckst · 2020-08-25T22:47:23Z

Don't forget to add yourself to AUTHORS and entry to CHANGES.

I think there's also an authors list in the docs conf.py so add yourself also.

xiki-tempula · 2020-08-26T09:32:04Z

@orbeckst I have changed the author list and embedded the figure.

orbeckst

Looking very good. Please see inline comments, especially about attributing code to alchemical-analysis if necessary (alchemical-analysis is MIT so that's all good)

Did you build the docs locally and do they look ok?

orbeckst · 2020-08-26T17:49:30Z

src/alchemlyb/visualisation/mbar_matrix.py

@@ -0,0 +1,81 @@
+"""Functions for Plotting the overlay matrix for the Mbar estimator.


Make it a title and write one short paragraph.

Use MBAR consistently (not Mbar)

You can also say that this code is based on alchemical-analysis.

src/alchemlyb/visualisation/mbar_matrix.py

orbeckst · 2020-08-26T17:51:35Z

src/alchemlyb/visualisation/mbar_matrix.py

+import numpy as np
+
+def plot_mbar_overlap_matrix(matrix, skip_lambda_index=[], ax=None):
+    '''Plot the MBar overlap matrix.


src/alchemlyb/estimators/mbar_.py

Co-authored-by: Oliver Beckstein <[email protected]>

xiki-tempula · 2020-08-26T18:48:04Z

@orbeckst Thank you for the suggestions. I have built the docs locally and it looks fine.

orbeckst

minor comments, please see inline, almost there!!

orbeckst · 2020-08-26T19:18:38Z

src/alchemlyb/visualisation/mbar_matrix.py

+
+    Parameters
+    ----------
+    matrix : DataFrame


Isn't this a numpy array?

orbeckst · 2020-08-26T19:19:20Z

src/alchemlyb/estimators/mbar_.py

@@ -45,6 +45,9 @@ class MBAR(BaseEstimator):
    states_ : list
        Lambda states for which free energy differences were obtained.

+    overlap_matrix: DataFrame


really a DataFrame... I thought that pymbar returns numpy arrays. please check and correct if necessary

xiki-tempula · 2020-08-26T19:27:39Z

@orbeckst Sorry for the omission. Data type corrected.

orbeckst · 2020-08-26T20:40:30Z

Thank you very much @xiki-tempula , great addition!

xiki-tempula added 4 commits August 24, 2020 16:36

mbar overlap matrix

dab5eac

depedency

b99bda9

py2 compatibility

6111a8e

backend error

bf80720

xiki-tempula added 2 commits August 24, 2020 17:06

bump coverage

c5ac34a

citation

8e55409

orbeckst requested changes Aug 25, 2020

View reviewed changes

xiki-tempula added 4 commits August 25, 2020 20:56

apply change

5f03a3e

fix test

c327215

fix test

095868e

fix test

0b998b9

fix visulisatiuon

a98027f

xiki-tempula added 2 commits August 26, 2020 10:33

Update CHANGES

5613966

Update CHANGES

505e0d6

orbeckst requested changes Aug 26, 2020

View reviewed changes

xiki-tempula and others added 2 commits August 26, 2020 19:38

Update src/alchemlyb/estimators/mbar_.py

453242c

Co-authored-by: Oliver Beckstein <[email protected]>

update docs

4492fab

orbeckst requested changes Aug 26, 2020

View reviewed changes

Change data type.

8cfd520

orbeckst approved these changes Aug 26, 2020

View reviewed changes

orbeckst self-assigned this Aug 26, 2020

orbeckst merged commit 7be2e41 into alchemistry:master Aug 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mbar overlap matrix plot #107

mbar overlap matrix plot #107

xiki-tempula commented Aug 24, 2020 •

edited

Loading

codecov bot commented Aug 24, 2020 •

edited

Loading

orbeckst left a comment

orbeckst Aug 25, 2020

xiki-tempula Aug 25, 2020

xiki-tempula Aug 25, 2020

orbeckst Aug 25, 2020

orbeckst Aug 25, 2020

orbeckst Aug 25, 2020

orbeckst Aug 25, 2020

orbeckst Aug 25, 2020

orbeckst Aug 25, 2020

orbeckst Aug 25, 2020

orbeckst Aug 25, 2020

orbeckst commented Aug 25, 2020 via email

orbeckst commented Aug 25, 2020

xiki-tempula commented Aug 26, 2020 •

edited

Loading

orbeckst left a comment

orbeckst Aug 26, 2020

orbeckst Aug 26, 2020

orbeckst Aug 26, 2020

xiki-tempula commented Aug 26, 2020

orbeckst left a comment

orbeckst Aug 26, 2020

orbeckst Aug 26, 2020

xiki-tempula commented Aug 26, 2020

orbeckst commented Aug 26, 2020

		@@ -0,0 +1,81 @@
		"""Functions for Plotting the overlay matrix for the Mbar estimator.

mbar overlap matrix plot #107

mbar overlap matrix plot #107

Conversation

xiki-tempula commented Aug 24, 2020 • edited Loading

codecov bot commented Aug 24, 2020 • edited Loading

Codecov Report

orbeckst left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

orbeckst commented Aug 25, 2020 via email

orbeckst commented Aug 25, 2020

xiki-tempula commented Aug 26, 2020 • edited Loading

orbeckst left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiki-tempula commented Aug 26, 2020

orbeckst left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiki-tempula commented Aug 26, 2020

orbeckst commented Aug 26, 2020

xiki-tempula commented Aug 24, 2020 •

edited

Loading

codecov bot commented Aug 24, 2020 •

edited

Loading

xiki-tempula commented Aug 26, 2020 •

edited

Loading