ENH and/or DOC: Better handling of atoms with resindex 0 in GROMACSAtom

[IAlibay]

Background

When importing a system from OpenMM you'll often find yourself with a case where your residue indices are set and 0-indexed.

GROMACSAtom understandably does not like this and will throw:

ValidationError: 1 validation error for GROMACSAtom
residue_index
  ensure this value is greater than 0 (type=value_error.number.not_gt; limit_value=0)

The solution here is simply shifting all your residue indices by one:

for at in interchange.topology.atoms:
     resid = int(at.metadata['residue_number'])
     at.metadata['residue_number'] = str(resid+1)

The documentation request

Documenting the need for 1-indexed residues and how to achieve them would be great.

The feature request

I take "respect the user's intent" as a good philosophy for things, but this may be the one case where handholding might be reasonable. Would adding some kind of "shift resids by one" flag in to_gromacs make sense? Even if not by default, it would save a few lines of code every time we have to think about doing this.

[mattwthompson]

Hmm ... not sure why that's not allowed. Are GROMACS residues 1-indexed by convention or by a standard (i.e. 0 won't be processed at all)? Agree that 0-indexed residue numbers are common enough to be considered a valid input of some sort.

Either way, this needs to be documented

[IAlibay]

I think resids and possibly even atomids are indexed by 1 (https://docs.mdanalysis.org/2.8.0-dev0/documentation_pages/coordinates/GRO.html#writing-gro-files
) - I know @lilyminium looked at this for TPRs a couple of years back (MDAnalysis/mdanalysis#2364) and probably would know more.

[mattwthompson]

Hmm ... my thinking now is shifting towards 1-index and warning the user is the best way to handle this. That means some roundtrips will be broken (arguably they should?) but models won't need to be updated.

[IAlibay]

Hmm ... my thinking now is shifting towards 1-index and warning the user is the best way to handle this. That means some roundtrips will be broken (arguably they should?) but models won't need to be updated.

Is (edit: lossless) roundtripping really an expected behaviour of Interchange?

I.e. would it be reasonable for users to expect that Interchange is the ground truth and that all exporters are lossy?

[mrshirts]

My expectation (I am not directly in charge of design!) that roundtrips should preserve information, in that it would still give the same energies in program X, but might not preserve formatting.

[mattwthompson]

These are good points - I agree that the most important thing to maintaining is the energy evaluations, where possible. Some exporters are certainly going to be lossy, and the information before export should be trusted more. Importers (though less developed) aim to take in as much information as possible from the input data, but as we've already seen that's not so simple.

Documenting in the right places that which details might be mutated is important, and I'm going for that in part of #1008.