re-enable gaff

[mikemhenry]

going to see if we can fix gaff support in an elegant way

[codecov-commenter]

:warning: Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 69.82%. Comparing base (b3fae57) to head (2596260).

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

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #334       +/-   ##
===========================================
+ Coverage   52.91%   69.82%   +16.91%     
===========================================
  Files           5        5               
  Lines         807      822       +15     
===========================================
+ Hits          427      574      +147     
+ Misses        380      248      -132     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[mattwthompson]

Just waiting on the patch release now https://github.com/openmm/openmm/issues/4572

[mattwthompson]

This is blocked by https://github.com/conda-forge/openmm-feedstock/pull/135 which might be fixed by https://github.com/conda-forge/openmm-feedstock/pull/137

[mattwthompson]

I'm seeing https://anaconda.org/conda-forge/openmm/files populated, and while the CDN might not be updated I'm re-kicking the jobs anyway. I'm offline for the day now, but will kick again in the morning if needed.

If this is green tomorrow, I'm going to merge and make a 0.13.0 based on this. Please do shout if there are other considerations we need to make @mikemhenry @ijpulidos

[mattwthompson]

Some actual failures here

[mikemhenry]

my $ is on ambertools crashing on osx-arm64, I will tinker with the macos versions of the runners to figure out what is going on

[mikemhenry]

The errors showing up now are ones I've seen reported by other people, I think it is still some ambertools weirdness where it fails to run ToolkitWrapper around AmberTools version 23.6 <class 'subprocess.CalledProcessError'> : Command '['antechamber', '-i', 'molecule.sdf', '-fi', 'sdf', '-o', 'charged.mol2', '-fo', 'mol2', '-pf', 'yes', '-dr', 'n', '-c', 'bcc', '-nc', '0.0']' returned non-zero exit status 1. which then messes up the openff-toolkit trying to figure out how to do partial charges ToolkitWrapper around The RDKit version 2024.03.4 <class 'openff.toolkit.utils.exceptions.ChargeMethodUnavailableError'> : partial_charge_method 'am1bcc' is not available from RDKitToolkitWrapper. Available charge methods are {'mmff94': {}, 'gasteiger': {}}

[mikemhenry]

looks like ambertools has some issues on macos-13, but works on macos-12 (older glibc version, x86) and macos-14 (newer glibc, osx-arm64) so that is surprising, but I feel confident that the changes made in this PR don't introduce any new regressions.

Thoughts @mattwthompson on these errors? Think we should just test on macos-14? Do you see any issues on openff repos that are using ambertools on macos-13?

FAILED openmmforcefields/tests/test_system_generator.py::TestSystemGenerator::test_parameterize_molecules_specified_during_create_system[openff-2.0.0] - ValueError: No registered toolkits can provide the capability "assign_partial_charges" for args "()" and kwargs "{'molecule': Molecule with name 'CAT-13f' and SMILES '[H][c]1[c]([H])[c]([Cl])[c]([H])[c](-[c]2[c]([H])[c]([H])[c]([H])[c]([C@]3([C]4([H])[C]([H])([H])[C]([H])([H])[C]([H])([H])[C]4([H])[H])[C](=[O])[N]([C]([H])([H])[H])[C]([N]([H])[H])=[N+]3[H])[c]2[H])[c]1[H]', 'partial_charge_method': 'am1bcc', 'use_conformers': None, 'strict_n_conformers': False, 'normalize_partial_charges': True, '_cls': <class 'openff.toolkit.topology.molecule.Molecule'>}"
Available toolkits are: [ToolkitWrapper around The RDKit version 2024.03.3, ToolkitWrapper around AmberTools version 23.6, ToolkitWrapper around Built-in Toolkit version None]
 ToolkitWrapper around The RDKit version 2024.03.3 <class 'openff.toolkit.utils.exceptions.ChargeMethodUnavailableError'> : partial_charge_method 'am1bcc' is not available from RDKitToolkitWrapper. Available charge methods are {'mmff94': {}, 'gasteiger': {}}
 ToolkitWrapper around AmberTools version 23.6 <class 'subprocess.CalledProcessError'> : Command '['antechamber', '-i', 'molecule.sdf', '-fi', 'sdf', '-o', 'charged.mol2', '-fo', 'mol2', '-pf', 'yes', '-dr', 'n', '-c', 'bcc', '-nc', '1.0']' returned non-zero exit status 1.

[mattwthompson]

I don't think we're using 13 anywhere; ideally we can test M1 and Intel chips but given the likelihood these issues are upstream packaging snafus and probably not issues with this code, I can live with only one version passing.

[mikemhenry]

Cool, I've got a meeting right now but if you @mattwthompson or @ijpulidos can give this another review, I can get a release cut and out onto conda-forge (I will also set CI to just test on osx-arm64)

[mattwthompson]

Alright, I ran through the diff twice and this does everything I hoped it would, including a brief changelog entry.

I gave the automation the ✅ so, with a little luck from the silicon deities, it should auto-merge within the hour!