Restructured WaterOrientationalRelaxation

[PicoCentauri]

trafficstars

Partly Fixes MDAnalysis/waterdynamics#22

Changes made in this Pull Request:

The WaterOrientationalRelaxation was restructured to follow the start, stop and step attributes of the AnalysisBase. The documentation was also improved and the example was adjusted to a pure water system. I also moved everything into the class documentation.

It seems that the results after my restructuring are okay but maybe somebody who knows how these correlation function should look takes a look at the results.

PR Checklist

• [x] Tests?
• [x] Docs?
• [ ] CHANGELOG updated?
• [x] Issue raised/referenced?

[pep8speaks]

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

• In the file package/MDAnalysis/analysis/waterdynamics.py:

Line 376:80: E501 line too long (82 > 79 characters) Line 430:78: W291 trailing whitespace

Comment last updated at 2021-04-30 21:06:00 UTC

[codecov[bot]]

Codecov Report

Merging #3247 (bf39ed5) into develop (f2745a9) will increase coverage by 0.03%. The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop    MDAnalysis/mdanalysis#3247      +/-   ##
===========================================
+ Coverage    92.81%   92.84%   +0.03%     
===========================================
  Files          170      170              
  Lines        22801    22810       +9     
  Branches      3239     3241       +2     
===========================================
+ Hits         21162    21179      +17     
+ Misses        1591     1583       -8     
  Partials        48       48              

Impacted Files	Coverage Δ
package/MDAnalysis/analysis/waterdynamics.py	96.01% <100.00%> (+0.42%)	:arrow_up:
package/MDAnalysis/topology/guessers.py	100.00% <0.00%> (ø)
package/MDAnalysis/core/topologyattrs.py	96.68% <0.00%> (ø)
package/MDAnalysis/topology/ITPParser.py	96.84% <0.00%> (ø)
package/MDAnalysis/coordinates/ParmEd.py	93.18% <0.00%> (+2.17%)	:arrow_up:
package/MDAnalysis/analysis/gnm.py	99.18% <0.00%> (+2.38%)	:arrow_up:

---

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 f2745a9...bf39ed5. Read the comment docs.

[orbeckst]

Are these changes are backwards-compatible?

[PicoCentauri]

In some way:

• t0, the argument for the initial step was silently ignored.
• dtmax, was the argument for the maximal step size considered for the correlation function. During my tests it seemed that only dtmax=2worked. Therefore, I removed the argument dtmax.

[orbeckst]

It sounds as if these changes get an entry under "Fixes" in the CHANGELOG. Then this PR can go in anytime after 2.0.0.

[orbeckst]

@alejob could you please review this PR? Thanks!

[alejob]

In some way:

• t0, the argument for the initial step was silently ignored.
• dtmax, was the argument for the maximal step size considered for the correlation function. During my tests it seemed that only dtmax=2worked. Therefore, I removed the argument dtmax.

Hi there!

dtmax is an important part of WOR analysis, this because is the time windows where the water molecules relax. Did this change pass the unitary test?

edit: Ok, I see that it has a new name.

[orbeckst]

@PicoCentauri is this PR something you might want to come back to?

[PicoCentauri]

Currently, I have to no time to look into this in great details. When we had the last round of nice feedback I had the feeling that the current state of the class is physically broken and that a transition to the AnalysisBase is not as straightforward as I thought in the beginning.

[IAlibay]

Currently, I have to no time to look into this in great details. When we had the last round of nice feedback I had the feeling that the current state of the class is physically broken and that a transition to the AnalysisBase is not as straightforward as I thought in the beginning.

@PicoCentauri this is on the 2.2.0 milestone, from this comment shall I assume that this is not going to make it? (I've not been keeping up with the review comments sorry).

If so, which future milestone shall I add this to? 2.3.0 or 3.0.0?

[PicoCentauri]

Probabaly 3.0.0