Update Amber force fields with latest versions from AmberTools 24

[epretti]

Featuring these new additions:

• ff14ipq (existed before in AmberTools but wasn't being converted)
• ff19ipq
• ff19SB (with CMAP)
• phosfb18
• phosaa19SB (with CMAP)
• DNA.OL21
• GAFF 2.2.20

Fixes:

• Can not use OPC/3 water with ff14SB #357
• Some GLYCAM residues had incorrect external bonds from some carboxylates, amides, and a few other places because the GLYCAM source files specify these external bonds incompletely and/or incorrectly, and Fix glycam ffxml and conversion script #180 didn't fix all of them. This should now be handled correctly by an update to ParmEd's algorithm for guessing them.
• lipid17.xml has unmerged lipids and lipid17_merged.xml has merged lipids. Previously lipid17.xml had merged lipids and lipid17_merged.xml had no residues at all. I think that was wrong and that how it is now is right, but please correct if needed.

A few notes:

• For complex reasons, a few impropers in ff19SB on non-central atoms have a different order from Amber. The improper tolerance had to be increased for one test. If we think this is a problem, the path of least resistance might be a one-off patch of these files: let me know if I should do this.
• We decided not to convert Lipid21 since it uses nonuniform 1-4 LJ scalings in a more complicated way than any other Amber force fields, and this is unsupported by ParmEd/OpenMM.
• A patched version of ParmEd (epretti/ParmEd:patched-for-amber-conversion) is needed for the conversion of some of the phosphorylated residue force fields, as well as GLYCAM due to the bug mentioned above.
• Some questionable syntax in Amber's source files gets rejected by ParmEd and so copies of some source files were made with the fixes. CMAP-containing FRCMOD files have also been copied and split for ff19SB/phosaa19SB.
• A bug in the Amber sources that previously necessitated separate test files for phospho10/phospho14 has been fixed, which is why I deleted one set of them.
• merge_lipids.py is removed since the one function in that script is in the main conversion script.
• Some outdated information in the README has been updated or removed to the best of my understanding.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.41%. Comparing base (7f0201b) to head (2af04a3).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #368   +/-   ##
=======================================
  Coverage   71.41%   71.41%           
=======================================
  Files           5        5           
  Lines         822      822           
=======================================
  Hits          587      587           
  Misses        235      235           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[epretti]

This turns out to be not quite ready to merge yet so I'll revert it to a draft for now. In particular, the main README and some docstrings and tests need to be updated, and GAFF 2.2.20 needs to be registered in the openmmforcefields code. Maybe this should be changed at some point to happen automatically based on what's in openmmforcefields/ffxml/amber/gaff/dat instead of being hardcoded.

Annoyingly, the code and tests that are trying to parse a "major" and "minor" version number from GAFF aren't valid here because 2.2.20 is a different format from all of the other GAFF versions and has 3 components. Maybe "2" can be the major version number and "2.20" can be the minor, since they look like strings?

[epretti]

Alright, this should be ready for review now. The GAFF-specific tests aren't turned on yet as I have a separate PR #371 for that.

[peastman]

This looks ok as far as I can tell, which once again isn't very far. Hopefully @mattwthompson and @mikemhenry can provide additional feedback. Is there any risk that replacing the protein- atom types with cmap- ones could create problems elsewhere?

[epretti]

In most cases it shouldn't, as ParmEd outputs class instead of type attributes for assigning all of the force field parameters. The only place this might be a problem is with GLYCAM, whose FFXML is converted to use types instead. I am having a lot of trouble testing this because there appears to be a bug in AmberTools' tleap where the presence of glycosylated residues sometimes causes it to fail to output any CMAP entries into the parameter file at all. It looks like the CMAPs are getting inserted correctly through OpenMM and the FFXML, but I want to make sure.

[mattwthompson]

I'm not able to really review this at the comprehensive level that's probably necessary. From the notes at the top of this page, it looks like the unresolved questions relate to several hacks that may or may not be necessary to work with ... odd things that already exist in Amber infrastructure. And bugs, unreleased code, etc. These smell collectively like a Sophie's choice - there are, putting it generously, compromises that need to be made to support all of these force fields, but just not supporting them is not without downsides.

What sort of things are you looking for feedback on? How could Mike, myself, and/or others best contribute to this situation?

[epretti]

Your assessments there are correct. Both this and the CHARMM update have involved increasing numbers of convoluted hacks to work around (1) idiosyncrasies in the source force fields and (2) shortcomings in ParmEd. (2) was the main pain point for CHARMM and (1) has been the main problem with Amber, but both of these issues have contributed to difficulties in both updates.

My main concern in asking for review of these updates is avoiding changes that cause the force fields to successfully parameterize a system but result in incorrect parameters getting assigned. As far as I can tell, based on the existing tests for the Amber conversion, the updates I have made to support the new force fields haven't generated any problems yielding these silent wrong answers. Having not looked at this code before, however, there's always the chance that I have missed something obvious that has introduced a subtle bug.

Ideally, someone who knows how this conversion worked for the existing force fields would be able to take a look at these changes. At this point, though, assuming that the people in John's group who originally worked on this conversion are no longer available, I think we can just hope for an additional set of eyes on the code to try to prevent any obvious issues from getting introduced.

[epretti]

After getting some help from the AmberTools mailing list, I was able to verify that the aforementioned CMAP issue was in fact a LEaP bug, and to find a workaround that allows validation of GLYCAM with ff19SB.

Also, the Amber manual shows in an example that GLYCAM should be loaded by LEaP before a protein force field. It doesn't mention at all that getting this the other way around can generate bogus dihedrals with in ff19SB. Getting the order right in LEaP produces sensible output, specifically for the HYP residues that were causing problems, and I've updated the override levels on HYP and CHYP so that the protein force field versions take precedence over the GLYCAM versions.

Now ff19SB should work correctly with GLYCAM. That should be all of the remaining outstanding issues that I am aware of.