added TopologyObject API

[lilyminium]

It'd be nice to have an easy way to add singular bonds/angles/dihedrals/impropers.

Changes made in this Pull Request:

To universe.Universe:

• added add_TopologyObject/s
• add_Bond/s is a convenience method
• added delete_TopologyObject/s (have not added delete_Bond/s)
• added refresh_fragments

To topologyattrs._Connections:

• added delete_bonds
• moved type checking to decorator

PR Checklist

• Tests?
• Docs?
  • What is the types keyword for in _Connection.add_bonds, and topology objects in general? Is order a reference to bond order -- what meaning does this have for angles?
• CHANGELOG updated?
• Issue raised/referenced?

By the way, I suppose you wouldn't be interested in 'tagging' TopologyObjects with extra information like force constants, etc. from topology files with that information?

[codecov]

Codecov Report

Merging #2382 into develop will decrease coverage by 0.04%.
The diff coverage is 95.95%.

@@             Coverage Diff             @@
##           develop    #2382      +/-   ##
===========================================
- Coverage    90.16%   90.12%   -0.05%     
===========================================
  Files          177      177              
  Lines        22553    22642      +89     
  Branches      2917     2933      +16     
===========================================
+ Hits         20335    20405      +70     
- Misses        1614     1631      +17     
- Partials       604      606       +2

Impacted Files	Coverage Δ
package/MDAnalysis/core/topology.py	100% <ø> (ø)	⬆️
package/MDAnalysis/core/groups.py	98.13% <100%> (-0.1%)	⬇️
package/MDAnalysis/core/topologyobjects.py	98.13% <100%> (-0.37%)	⬇️
package/MDAnalysis/core/topologyattrs.py	98.11% <94.11%> (+0.49%)	⬆️
package/MDAnalysis/core/universe.py	95.44% <96.72%> (+0.2%)	⬆️
package/MDAnalysis/analysis/base.py	93.57% <0%> (-3.67%)	⬇️
package/MDAnalysis/coordinates/TRZ.py	82.15% <0%> (-1.49%)	⬇️
package/MDAnalysis/analysis/lineardensity.py	62.38% <0%> (-0.92%)	⬇️
package/MDAnalysis/lib/util.py	87.68% <0%> (-0.88%)	⬇️
package/MDAnalysis/analysis/gnm.py	95.38% <0%> (-0.77%)	⬇️
... and 6 more

---

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 72ce35a...a14dd93. Read the comment docs.

[lilyminium]

Hopefully fixes #2386. Added a _SymmetricConnection class for Bonds, Angles and Dihedrals with type checking and values where the first atom index is always smaller than the last. Added type checking to Impropers and a test to ensure that u.impropers returns impropers with the same indices as u._topology.impropers.

[orbeckst]

By the way, I suppose you wouldn't be interested in 'tagging' TopologyObjects with extra information like force constants, etc. from topology files with that information?

Do you mean similar to how we add Attributes in a flexible manner? Seems conceptually simple to add something like "force_constant", "equilibrium_value", "ryckaert_bellemans_coefficients", ... As long as these are "tags" and we don't really have to make sure that they are consistent between everything, it might be a convenient and extensible way to annotate with force field information.

If we want to be consistent then this gets harder...

[lilyminium]

GROMACS also can have multiple definitions of the same bonds etc. that are additive, which complicates things. :(

[lilyminium]

This last commit addresses #2392, but I don't know why it was written like that in the first place, so possibly not the best idea. There are more test offerings to please the codecov overlord.

[orbeckst]

I would factor out a small PR for #2392 if possible – I could review that one.

It's harder for me to substantially review the bigger part of adding the API. In principle the API looks good to me but I'd want a few more eyes on it from people who have been working on that part of the code. Feel free to ping people – ask for reviews and remind them occasionally.

[zemanj]

I like how you deal with fragment invalidation by making the new add_... methods members of Universe. This PR is a major step forward with respect to supporting dynamic topologies.
I only have some minor requests regarding method names and docs.
I didn't have time to review the tests yet, so that's still on the todo list.

[lilyminium]

Thanks for the review, @zemanj!

[richardjgowers]

Ok I did a little tidying and added back the improper dihedral stuff, but I think this is good to go now

[lilyminium]

Thanks @richardjgowers!

[lilyminium]

self = <MDAnalysisTests.topology.test_top.TestPRMParser object at 0x7f7087f38860>
top = <MDAnalysis.core.topology.Topology object at 0x7f7087f4a390>

        def test_improper_atoms_bonded(self, top):
            vals = top.bonds.values
            for imp in top.impropers.values:
                for b in ((imp[0], imp[2]), (imp[1], imp[2]), (imp[2], imp[3])):
    >               assert (b in vals) or (b[::-1] in vals)
    E               assert ((31, 43) in [(10, 11), (10, 12), (4, 6), (4, 10), (0, 4), (25, 26), ...] or (43, 31) in [(10, 11), (10, 12), (4, 6), (4, 10), (0, 4), (25, 26), ...])

    testsuite/MDAnalysisTests/topology/test_top.py:127: AssertionError

Ah, this is the test that kicked off the symmetric improper dihedral ordering issue in #2386. It might be better to remove this test; if the central atom can be either first or third, you can no longer assume bonding anyway. Extended it to check both forwards and backwards.

[richardjgowers]

Looks good to me, @zemanj are you happy with this?

[lilyminium]

For the identical topology case I'd sooner make them use u2.add_angles(u1.angles.to_indices())

That doesn't seem too inconvenient. The code now ensures you cannot add or delete a TopologyGroup, list of TopologyObjects, or list of AtomGroups, from a different Universe.

Comments addressed