Openff-toolkit: Base class for TopologyMolecule/Bond/Atom and Molecule/Bond/Atom

[andrrizzi]

If I understand the rationale for the current design, we currently keep distinct topology.TopologyMolecule/Bond/Atom and molecule.Molecule/Bond/Atom because we don't want to include all the information in Molecule in the Topology as well (e.g. partial charges and conformers). Nevertheless, the overlap between the two set of classes is quite large so I'm wondering if it makes sense to have a common set of base classes for both to make the interface uniform.

[jchodera]

@andrrizzi : The distinction between Molecule and TopologyMolecule is

• Molecule stores all the information associated about a unique, real chemical molecular species
• TopologyMolecule just encapsulates information about the "image" of that real molecule mapped into a set of atoms in the Topology object

Their APIs should be very similar, with the TopologyMolecule providing additional information about the mapping of atoms in the Topology and the naming of that molecule in the topology, but many of the properties should be pass-through to Molecule (but immutable).

If there was a way to simplify the design by deriving from a common base class, I'm all for it!

We do want to keep in mind that the API for Molecule and Topology should be our new replacement for all the limitations of the OpenMM Topology molecule, so it will eventually need to be flexible enough to allow perses-style editing of molecules and topologies on the fly. Ideally, the level of abstract we pick for these objects will enable a wide variety of applications to be easily built on top of them, beyond just forcefield parameterization!

[j-wags]

As Andrea and I discussed earlier today, I think this would be a good idea in terms of making the API more consistent, but we'd have to be careful during implementation. Some queries, like TopologyAtom.atomic_number simply resolve to self._atom.atomic_number. However, things may get hairy when we do TopologyAtom.bonds, which requires mapping back to the reference atom, then collecting its bonds, then generating TopologyBonds for each one. It's not impossible, but it may be convoluted.

@andrrizzi also brought up a good point, which is that the current implementation tries to be as memory-efficient as possible, at the detriment of becoming more difficult to use. This memory efficiency may only be on the order of a factor of 2. The reasoning is, at the end of the day, we're going to generate an OpenMM system with N atoms in memory anyway, so how bad could it be to have _two_ data structures of N atoms?

[jchodera]

We should definitely prioritize ease of use over memory efficiency!

[andrrizzi]

Agreed! For the upcoming release, it's probably wise to keep things as they are, but I'll see if I can find a way to simplify a bit the API while keeping all the existing advantages after that.