Optimized and refactored common compound code.

[mnmelo]

Fixes #3000

Changes made in this Pull Request:

• Implements the ideas in Use of compounds involves excessive sorting and fancy-indexing of AtomGroups and compound indices. #3000;
• Compound code in core/group.py refactored (more DRY now);
• Caching functionally implemented, but cache invalidation still to do.

Performance benchmarks, in the discussion of #3000, are quite promising, especially with caching.

Problem

Cached compound masks work great and can give large speedups, but invalidating them is tough: if I have an AtomGroup with cached masks for segments and the user moves a residue between segments those cached masks should no longer be valid, but how to notify an AtomGroup of something occurring at the ResidueGroup/SegmentGroup level?

Pointers appreciated.

PR Checklist

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

[pep8speaks]

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

• In the file benchmarks/benchmarks/ag_methods.py:

Line 329:1: W293 blank line contains whitespace
Line 333:1: W293 blank line contains whitespace
Line 348:1: W293 blank line contains whitespace
Line 362:1: W293 blank line contains whitespace
Line 366:1: W293 blank line contains whitespace
Line 369:1: W293 blank line contains whitespace

• In the file package/MDAnalysis/core/groups.py:

Line 816:36: E127 continuation line over-indented for visual indent
Line 819:36: E127 continuation line over-indented for visual indent
Line 1711:47: E128 continuation line under-indented for visual indent

Comment last updated at 2021-02-08 05:22:51 UTC

[codecov]

Codecov Report

Merging #3005 (a639166) into develop (258a73e) will decrease coverage by 0.06%.
The diff coverage is 98.79%.

@@             Coverage Diff             @@
##           develop    #3005      +/-   ##
===========================================
- Coverage    93.14%   93.08%   -0.07%     
===========================================
  Files          171      171              
  Lines        22750    22706      -44     
  Branches      3218     3209       -9     
===========================================
- Hits         21191    21135      -56     
- Misses        1502     1514      +12     
  Partials        57       57              

Impacted Files	Coverage Δ
package/MDAnalysis/lib/_cutil.pyx	100.00% <ø> (ø)
package/MDAnalysis/core/groups.py	98.43% <98.79%> (-0.16%)	⬇️
package/MDAnalysis/coordinates/H5MD.py	91.08% <0.00%> (-3.47%)	⬇️
package/MDAnalysis/core/universe.py	96.83% <0.00%> (-0.98%)	⬇️

---

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 258a73e...a639166. Read the comment docs.

[mnmelo]

In #2376 I propose a solution to the cache invalidation problem, which is to cache at the Universe level.

[richardjgowers]

I like the DRY changes in this PR a lot. I can't see where the caching is, has this been removed for now? If so I'm happy to merge these changes.

[mnmelo]

I have dropped the caching for this PR, so it's more contained and easier to review. (Also, the caching invalidation mechanism is what is still up for discussion).

Gonna clean up these lingering bugs and update soon (should've tested locally 😬).

[mnmelo]

Ok, this is ready for review. I guess codecov hit a roadbump or something because only groups.py was touched and diff coverage is almost 100% there.

[mnmelo]

To get my feet wet in asv I also implemented the performance benchmarks I had posted in #3000. I really don't know what the status is on asv usage, so guidance is appreciated.

[mnmelo]

Ping @richardjgowers. I think this is ready to merge. There seem to be some inconsistencies in Codecov computation triggering a fail there.
Also, this PR removes a couple hundred lines of repeated code, and the coverage may indeed go down just because of that, even if all remaining lines are as covered as before.

[richardjgowers]

@mnmelo thanks!