Hydrogen Bond Lifetime in waterdynamics.hydrogenbonds #2791

bieniekmateusz · 2020-06-25T15:21:33Z

Changes made in this Pull Request:

The standalone autocorrelation function is now used to calculate the hydrogen bond lifetime in the new implementation of the hydrogen bond analysis class.
We deprecate the waterdynamics.HydrogenBondLifetime because of HydrogenBondLifetimes (Autocorrelation) has an increasing value #2247.
This function can also replace the hbonds.hbond_autocorrel module. However, we haven't deprecated this module here as that is being addressed by PR Remove deprecated hbonds.hbond_analysis #2746
Basic unit tests added to check the integration of the separate components

Co-authored-by: @p-j-smith paul.smith@kcl.ac.uk

PR Checklist

Tests?
Docs?
CHANGELOG updated?
Issue raised/referenced?

…ydrogen bond implementation. Fixes MDAnalysis#2547 and we depracate the waterdynamic.HydrogenBondLifetime because of MDAnalysis#2247. co-authored-by: p-j-smith <paul.smith@kcl.ac.uk>

codecov · 2020-06-25T16:24:23Z

Codecov Report

Merging #2791 into develop will increase coverage by 0.02%.
The diff coverage is 98.00%.

@@             Coverage Diff             @@
##           develop    #2791      +/-   ##
===========================================
+ Coverage    92.19%   92.22%   +0.02%     
===========================================
  Files          184      184              
  Lines        24072    24177     +105     
  Branches      3122     3133      +11     
===========================================
+ Hits         22194    22298     +104     
- Misses        1813     1814       +1     
  Partials        65       65

Impacted Files	Coverage Δ
...DAnalysis/analysis/hydrogenbonds/hbond_analysis.py	`98.63% <97.43%> (-0.46%)`	⬇️
package/MDAnalysis/analysis/waterdynamics.py	`96.14% <100.00%> (+0.01%)`	⬆️
package/MDAnalysis/lib/correlations.py	`97.61% <100.00%> (ø)`
util.py	`88.10% <0.00%> (+0.08%)`	⬆️
coordinates/base.py	`94.82% <0.00%> (+0.28%)`	⬆️
auxiliary/base.py	`91.31% <0.00%> (+0.56%)`	⬆️

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 17969df...93b6707. Read the comment docs.

orbeckst

Thank you !

Various doc/deprecation comments.
add an entry to CHANGELOG
diff coverage is low; can you check that most of the changes are covered by tests?

package/MDAnalysis/analysis/waterdynamics.py

package/MDAnalysis/lib/correlations.py

package/MDAnalysis/analysis/waterdynamics.py

Co-authored-by: Oliver Beckstein <orbeckst@gmail.com>

co-authored-by: p-j-smith <paul.smith@kcl.ac.uk>

hmacdope · 2020-06-28T09:42:17Z

Hey all, looking great!

Not sure how applicable to this use case, but just thought I would let you know that tidynamics, which is now an optional dependency, provides an ACF computation option using the FFT based fast correlation algorithm.

Perhaps some of the computation can be outsourced? :)

I know im a bit late, sorry.

bieniekmateusz · 2020-06-28T10:09:07Z

@hmacdope Thanks for pointing this out. I'll look into it!

co-authored-by: p-j-smith <paul.smith@kcl.ac.uk>

bieniekmateusz · 2020-06-28T14:56:09Z

After further thought, together with @p-j-smith we are going to expand this pull request to make it easier to obtain hydrogen bond lifetimes. Please don't merge yet.

Thanks

co-authored-by: p-j-smith <paul.smith@kcl.ac.uk>

The `between` keyword can be used to specify pairs of groups between which hydrogen bonds will be calculated. Hydrogen bonds found other pairs of atom groups will be discarded. Added test cases for filtering hydrogen bonds. co-authored-by: p-j-smith <paul.smith@kcl.ac.uk>

p-j-smith · 2020-06-29T17:18:24Z

In adding the hydrogen bond lifetime functionality we realised it was not possible to find, say, protein-water hydrogen bonds without also finding protein-protein and water-water-hydrogen bonds. This would therefore make the HBL function of limited use if one wanted to know the lifetime of protein-water hydrogen bonds. We have therefore added a between argument to HydrogenBondsAnalysis to make it possible to specify pairs of atoms groups between which hydrogen bonds will be formed. For example between=["protein", "resname SOL"] will find only protein-water hydrogen bonds.

This is now in keeping with both the VMD hbonds plugin and MDAnalysis.analysis.hbobds.hbond_analysis.HydrogenBondsAnalysis, although we thought a between keyword, rather than sel1 and sel2, would be preferable. This is because it allows for more flexibility, such as passing two pairs of selections, e.g between=[["protein", "protein"], ["protein", "resname SOL"]] will find all protein-protein and protein-water hydrogen bonds but not water-water hydrogen bonds.

We have added test cases for this new argument and the interface is backwards compatible - the default is between=None, in which case all hydrogen bonds are found.

We also plan to add an of keyword to the HBL lifetime function, e.g hbonds.lifetime(of=["resname ASP", "resname SOL"]) would calculate the hydrogen bond lifetime of hydrogen bonds between aspartic acid and water. One could therefore use the between keyword to find all protein-water hydrogen bonds, and subsequently use hbonds.lifetime(of) to calculate the HBL of particular specific hydrogen bonds. We don't want to bloat this PR too much and so we'll leave this of argument for another time.

And thanks @hmacdope for pointing us toward tidynamics. The autocorrelation function we're using to calculate the HBL is also used by MDAnalysis.analysis.waterdynamics to calculate the survival probability of water around proteins, and so it would probably be best to make any changes to the autocorrelation function in a separate PR. But it's definitely something to look into!

orbeckst

Nice work. I primarily have comments on the documentation, see inline.

package/CHANGELOG

package/MDAnalysis/analysis/hydrogenbonds/hbond_analysis.py

package/MDAnalysis/analysis/waterdynamics.py

package/MDAnalysis/lib/correlations.py

testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py

co-authored-by: p-j-smith <paul.smith@kcl.ac.uk>

pep8speaks · 2020-07-05T16:11:17Z

Hello @bieniekmateusz! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-07-07 13:15:16 UTC

Fixed the line length and whitespace issues

Previously, a selection involving water and protein was used, but the example suggested hydrogen bonds between a ligand and protein would be found.

Sphinx docs were previously not building

orbeckst

minor comments, sorry to be brief

package/MDAnalysis/analysis/hydrogenbonds/hbond_analysis.py

package/MDAnalysis/analysis/waterdynamics.py

testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py

Co-authored-by: Oliver Beckstein <orbeckst@gmail.com>

co-authored-by: p-j-smith <paul.smith@kcl.ac.uk>

…into hbondautocorr

co-authored-by: p-j-smith <paul.smith@kcl.ac.uk>

bieniekmateusz · 2020-07-07T13:16:32Z

Please let us know if you spot anything else! Thanks!

orbeckst · 2020-07-12T08:27:38Z

Thank you @bieniekmateusz and @p-j-smith – sorry it took me a while to get back to the PR.

- see #2547 and PR #2791 - will be replaced by MDAnalysis.analysis.hydrogenbonds.HydrogenBondAnalysis.lifetime (in 2.0.0) - add test for deprecation warning - update CHANGELOG

orbeckst · 2020-07-12T09:02:18Z

@bieniekmateusz @p-j-smith – You know what? We should have removed wateranalysis.HydrogenBondLifetimes in this PR because this PR is really for 2.0.0 when the class will have been removed. Sorry, I didn't quite realize it during review. I backported the deprecation to #2798 (for 1.0.1). Can you please submit another PR where you remove the deprecated functionality and add a CHANGELOG entry under Changes saying so? Thanks!

co-authored-by: p-j-smith <paul.smith@kcl.ac.uk>

* remove the deprecated waterdynamics.HydrogenBondLifetimes class, see PR #2791 for more info. * update waterdynamics docs and link to the replacement, hydrogenbonds.hbond_analysis.HydrogenBondAnalysis.liftetime * corrected doc formatting in hydrogenbonds.hbond_analysis * update CHANGELOG co-authored-by: p-j-smith <paul.smith@kcl.ac.uk>

* Fixes MDAnalysis#2547 * The standalone autocorrelation function is now used to calculate the hydrogen bond lifetime in the new implementation of the hydrogen bond analysis class. * Added ability to select groups between which to find hydrogen bonds.: new parameter "between" for hydrogen binding (backwards compatible): The `between` keyword can be used to specify pairs of groups between which hydrogen bonds will be calculated. Hydrogen bonds found other pairs of atom groups will be discarded. * We deprecate the waterdynamics.HydrogenBondLifetime class because of MDAnalysis#2247. * Basic unit tests added to check the integration of the separate components * add docs with example * update CHANGELOG co-authored-by: p-j-smith <paul.smith@kcl.ac.uk>

…alysis#2842) * remove the deprecated waterdynamics.HydrogenBondLifetimes class, see PR MDAnalysis#2791 for more info. * update waterdynamics docs and link to the replacement, hydrogenbonds.hbond_analysis.HydrogenBondAnalysis.liftetime * corrected doc formatting in hydrogenbonds.hbond_analysis * update CHANGELOG co-authored-by: p-j-smith <paul.smith@kcl.ac.uk>

Adapting the autocorrelation and intermittency functions in the new h…

a94d350

…ydrogen bond implementation. Fixes MDAnalysis#2547 and we depracate the waterdynamic.HydrogenBondLifetime because of MDAnalysis#2247. co-authored-by: p-j-smith <paul.smith@kcl.ac.uk>

orbeckst requested changes Jun 25, 2020

View reviewed changes

package/MDAnalysis/analysis/waterdynamics.py Outdated Show resolved Hide resolved

package/MDAnalysis/lib/correlations.py Show resolved Hide resolved

package/MDAnalysis/analysis/waterdynamics.py Outdated Show resolved Hide resolved

bieniekmateusz and others added 3 commits June 26, 2020 13:34

Update package/MDAnalysis/analysis/waterdynamics.py

7b7badc

Co-authored-by: Oliver Beckstein <orbeckst@gmail.com>

Coverage tests and refactoring for PR MDAnalysis#2791

0b9a399

co-authored-by: p-j-smith <paul.smith@kcl.ac.uk>

Coverage tests (PR MDAnalysis#2791)

42b9ad3

co-authored-by: p-j-smith <paul.smith@kcl.ac.uk>

Parameter "between" for hydrogen binding.

11ed00c

co-authored-by: p-j-smith <paul.smith@kcl.ac.uk>

bieniekmateusz and others added 2 commits June 29, 2020 16:24

Missing function rename

d2b947a

co-authored-by: p-j-smith <paul.smith@kcl.ac.uk>

orbeckst requested changes Jul 4, 2020

View reviewed changes

Addressing feedback in PR MDAnalysis#2791

b0d341b

co-authored-by: p-j-smith <paul.smith@kcl.ac.uk>

Paul Smith added 6 commits July 6, 2020 13:26

Silence pep8speaks

edb717b

Fixed the line length and whitespace issues

Fix between argument examples.

f6c3e2a

Previously, a selection involving water and protein was used, but the example suggested hydrogen bonds between a ligand and protein would be found.

Merge branch 'develop' into hbondautocorr

502fb5d

Fix docstring formatting for pep8

0698e22

Fix tests that I broke

d16d227

Fixed example in docs

33f260c

Sphinx docs were previously not building

orbeckst requested changes Jul 6, 2020

View reviewed changes

orbeckst self-assigned this Jul 6, 2020

orbeckst added this to the 1.0.x milestone Jul 6, 2020

bieniekmateusz and others added 4 commits July 7, 2020 13:58

Update package/MDAnalysis/analysis/waterdynamics.py

9b087a4

Co-authored-by: Oliver Beckstein <orbeckst@gmail.com>

Addressing feedback in PR MDAnalysis#2791

904e75d

co-authored-by: p-j-smith <paul.smith@kcl.ac.uk>

Merge branch 'hbondautocorr' of github.com:bieniekmateusz/mdanalysis …

d648d0f

…into hbondautocorr

Addressing feedback in PR MDAnalysis#2791

93b6707

co-authored-by: p-j-smith <paul.smith@kcl.ac.uk>

orbeckst approved these changes Jul 12, 2020

View reviewed changes

orbeckst merged commit d266b0b into MDAnalysis:develop Jul 12, 2020

orbeckst modified the milestones: 1.0.x, 2.0 Jul 12, 2020

bieniekmateusz added a commit to bieniekmateusz/mdanalysis that referenced this pull request Jul 12, 2020

Removing the deprecated module, see PR MDAnalysis#2791 for more info.

cf49609

co-authored-by: p-j-smith <paul.smith@kcl.ac.uk>

bieniekmateusz mentioned this pull request Jul 12, 2020

Removing the deprecated module, see PR #2791 for more info. #2842

Merged

4 tasks

bieniekmateusz added a commit to bieniekmateusz/mdanalysis that referenced this pull request Jul 12, 2020

Correcting user documentation PR MDAnalysis#2791.

35d2a8c

co-authored-by: p-j-smith <paul.smith@kcl.ac.uk>

lilyminium mentioned this pull request Oct 13, 2020

deprecate hydrogen bond analysis in water bridge analysis #2913

Merged

4 tasks

IAlibay mentioned this pull request Oct 15, 2020

HydrogenBondAutoCorrel uses deprecated features of numpy #2987

Closed

fiona-naughton added enhancement Component-Analysis labels Sep 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hydrogen Bond Lifetime in waterdynamics.hydrogenbonds #2791

Hydrogen Bond Lifetime in waterdynamics.hydrogenbonds #2791

bieniekmateusz commented Jun 25, 2020 •

edited

Loading

codecov bot commented Jun 25, 2020 •

edited

Loading

orbeckst left a comment

hmacdope commented Jun 28, 2020 •

edited

Loading

bieniekmateusz commented Jun 28, 2020

bieniekmateusz commented Jun 28, 2020 •

edited

Loading

p-j-smith commented Jun 29, 2020

orbeckst left a comment

pep8speaks commented Jul 5, 2020 •

edited

Loading

orbeckst left a comment

bieniekmateusz commented Jul 7, 2020

orbeckst commented Jul 12, 2020

orbeckst commented Jul 12, 2020

Hydrogen Bond Lifetime in waterdynamics.hydrogenbonds #2791

Hydrogen Bond Lifetime in waterdynamics.hydrogenbonds #2791

Conversation

bieniekmateusz commented Jun 25, 2020 • edited Loading

PR Checklist

codecov bot commented Jun 25, 2020 • edited Loading

Codecov Report

orbeckst left a comment

Choose a reason for hiding this comment

hmacdope commented Jun 28, 2020 • edited Loading

bieniekmateusz commented Jun 28, 2020

bieniekmateusz commented Jun 28, 2020 • edited Loading

p-j-smith commented Jun 29, 2020

orbeckst left a comment

Choose a reason for hiding this comment

pep8speaks commented Jul 5, 2020 • edited Loading

Comment last updated at 2020-07-07 13:15:16 UTC

orbeckst left a comment

Choose a reason for hiding this comment

bieniekmateusz commented Jul 7, 2020

orbeckst commented Jul 12, 2020

orbeckst commented Jul 12, 2020

bieniekmateusz commented Jun 25, 2020 •

edited

Loading

codecov bot commented Jun 25, 2020 •

edited

Loading

hmacdope commented Jun 28, 2020 •

edited

Loading

bieniekmateusz commented Jun 28, 2020 •

edited

Loading

pep8speaks commented Jul 5, 2020 •

edited

Loading