openmmforcefields icon indicating copy to clipboard operation
openmmforcefields copied to clipboard

Convert all amber ions; add OPC and OPC3 waters

Open aizvorski opened this issue 3 years ago • 8 comments
trafficstars

This converts one-to-one all Amber ion frcmod files (from AmberTools 22) into OpenMM xml files with the same names and the same contents. Implements what is proposed in https://github.com/openmm/openmm/issues/3663 Moved here based on feedback from @peastman

Run the conversion:

python ./amber/convert_amber_ions.py ~/anaconda3/pkgs/ambertools-22.0-py38h6177452_1

The new xml files end up in amber/ffxml/ions/

This does not change any of the existing ion conversion, and also does not combine water and ions in a single file. The intention is to have the water+ions be made by using <Include file="..."/> mechanism in the xml, rather than by duplicating the contents; but that is a future change, at the moment this doesn't change any existing files. It also doesn't use yaml - since the idea is to convert everything, there is nothing to configure.

Also, adds OPC and OPC3 waters, with the appropriate ion sets. The waters are from https://github.com/openmm/openmm/pull/3654 and the opc_standard.xml file shows the proposed method for combining water and ions.

Feedback requested:

  • How to test? validate_water_ion() has a nice unit test for water+ions, but not obvious how to run that outside of a yaml recipe
  • Any downsides to the suggested method of including a water and ions xml files to make the *standard.xml files?

aizvorski avatar Sep 28 '22 19:09 aizvorski

Thanks so much for contributing this! We're currently on AmberTools 20.15---would it make sense to include this in a full update of all Amber force fields to AmberTools 22?

jchodera avatar Sep 29 '22 23:09 jchodera

Thanks so much for contributing this! We're currently on AmberTools 20.15---would it make sense to include this in a full update of all Amber force fields to AmberTools 22?

@jchodera Welcome! About AmberTools 22 - I'm game to give it a try, but I don't know enough about the conversion process to easily debug any potential issues that pop up if it doesn't work out of the box.

If it's okay, perhaps we can do it in two stages - first get the updated water and ions (so we can get OPC, OPC3+ions, and the newer L&M ions for TIP3P-FB and TIP4P-FB in next openmm 8.x release), then update everything else?

aizvorski avatar Oct 01 '22 03:10 aizvorski

@jchodera I saw your https://github.com/openmm/openmmforcefields/pull/184 - did AmberTools 21 work okay with the existing conversion setup?

aizvorski avatar Oct 01 '22 03:10 aizvorski

Any idea what has changed between 20 and 22?

peastman avatar Oct 01 '22 04:10 peastman

Any idea what has changed between 20 and 22?

@peastman I don't know exactly what the release notes say, but from a quick diff looks like the main additions are:

  • lipids21 (move 14 and 17 to old)
  • Li&Merz ion sets for opc, opc3, fb3 and fb4 (these become the defaults for all water models for which they're available)
  • opc3pol
  • tip4pd-a99SB-disp (also btw removes the hardwired tip3p from some of the older protein ff, probably to enable using them with either tip3p or tip4pd)
  • chromophores 2016 and 2022

What hasn't changed (at all, as far as I can tell)

  • ff14SB, ff19SB, ff99SBildn
  • common waters (tip3p etc)
$ diff -ur anaconda3/pkgs/ambertools-20.15-py38hd01091a_2/dat/leap/parm/ anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/ | grep Only
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: frcmod.ff15ipq-19F
Only in anaconda3/pkgs/ambertools-20.15-py38hd01091a_2/dat/leap/parm/: frcmod.ions1lm_126_hfe_opc
Only in anaconda3/pkgs/ambertools-20.15-py38hd01091a_2/dat/leap/parm/: frcmod.ions1lm_126_iod_opc
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: frcmod.ionslm_1264_fb3
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: frcmod.ionslm_1264_fb4
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: frcmod.ionslm_1264_opc
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: frcmod.ionslm_1264_opc3
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: frcmod.ionslm_126_fb3
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: frcmod.ionslm_126_fb4
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: frcmod.ionslm_126_opc
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: frcmod.ionslm_126_opc3
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: frcmod.ionslm_hfe_fb3
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: frcmod.ionslm_hfe_fb4
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: frcmod.ionslm_hfe_opc
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: frcmod.ionslm_hfe_opc3
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: frcmod.ionslm_iod_fb3
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: frcmod.ionslm_iod_fb4
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: frcmod.ionslm_iod_opc
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: frcmod.ionslm_iod_opc3
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: frcmod.opc3pol
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: frcmod.opc3pol_HMR4fs
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: frcmod.tip4pd-a99SB-disp
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: frcmod.xFPchromophores.2016
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: frcmod.xFPchromophores.2022
Only in anaconda3/pkgs/ambertools-22.0-py38h6177452_1/dat/leap/parm/: lipid21.dat

aizvorski avatar Oct 01 '22 16:10 aizvorski

@jchodera @peastman Could you please let me know what is necessary to do before we can get this PR merged?

I'm hoping to have the new ion sets in the next openmm release, to go with the shiny new water models :)

aizvorski avatar Oct 15 '22 19:10 aizvorski

@jchodera I think that's mainly a question for you.

I notice the CI builds are failing. It looks like that's due to the lack of an OpenEye license. Is that something that needs to be fixed in the project settings?

peastman avatar Oct 17 '22 22:10 peastman

If the license is set up in the way that I'm familiar with, it is by design not accessible from forks and should remain that way, unfortunately. I've pushed these commits to an upstream branch and #242 at least to see if CI passes, though I'm not sure what is tested that would cause CI to fail.

I'd be happy to offer help but I'm not comfortable enough with Amber internals to review. Agree John would be the best approver here; you might be better able to get his attention through other channels.

mattwthompson avatar Oct 17 '22 23:10 mattwthompson

@aizvorski : Can you take a look at https://github.com/openmm/openmmforcefields/pull/242 and see if it looks good to merge to supplant this pR?

jchodera avatar Oct 21 '22 02:10 jchodera

Merged as https://github.com/openmm/openmmforcefields/pull/242

aizvorski avatar Oct 22 '22 18:10 aizvorski