[DNM] Pinning ambertools version to <= 22

[ijpulidos]

Ambertools 23 changed the behavior and it is causing failures with our tests. This pins the version for now so we can get a release while we find a solution for #280

Resolves #286

[ijpulidos]

I'll also be dropping testing with python 3.8 in our CI in this same PR.

[mattwthompson]

This failure

FAILED openmmforcefields/tests/test_template_generators.py::TestSMIRNOFFTemplateGenerator::test_energies - ValueError: Can't find specified SMIRNOFF force field (opc3) in install paths

stems from OPC being shipped by openff-forcefields. You can cherry-pick a band-aid from here: 4d7c50f or just leave it for #287

[ijpulidos]

@mattwthompson Thanks! I think resolving #287 before any other PR makes sense, unless folks have some objection about it.

Any ideas about this one https://github.com/openmm/openmmforcefields/actions/runs/5524287757/jobs/10076389477#step:9:752 ? I'm clueless about that one

[mattwthompson]

Any ideas about this one

The error is confusing, but it's actually the same issue as when water models are included in SMIRNOFFTemplateGenerator.INSTALLED_FORCEFIELDS or whatever that variable is called. The water models don't include AM1-BCC, just library charges for water, so the simplest of molecules fail charge assignment. The error is not useful (there should be a way to capture which force field failed and bubble that up, I just haven't done the work) and I think it might have been @mikemhenry who did the detective work somewhere else in this repo a few months ago.

[ijpulidos]

@mattwthompson As far as I can tell it's happening with the opc3 force field and the C=O molecule, as per https://github.com/openmm/openmmforcefields/actions/runs/5524287757/jobs/10076389477#step:9:601

[ijpulidos]

So I guess it doesn't make sense to parametrize a non-water molecule with the opc3 force field.

[mikemhenry]

#273 Hmmm that's I think where I noticed it first, trying to remember what I thought that was....

[mattwthompson]

Right - I'm suggesting that openmmforcefields does not include water models in its list of small molecule force fields at all. Probably having an allow list with openff-[0-9].[0-9].[0-9].offxml type of regex would be a safer approach when collecting force fields

[mikemhenry]

Okay that isn't a bad idea, but will there be an issue if a user supplies their own SMIRNOFF xml (from like bespoke fit)?

[mattwthompson]

That's fair - #288 would be a workaround but not the most elegant choice. You could instead just filter out OPC/TIP4P/etc. in the test, since I think it just naively runs through all available force fields whether or not it makes sense to try them on a molecule. That would prevent restrictions on what gets fed through the get_available_force_fields machinery.

If a user submits a bug report that TIP4P isn't parametrizing aspirin, I think we'd close that hypothetical issue as invalid, after all.

Anyway, you'll probably just want to go for whatever gets CI passing again and most promptly, and I'll update #287 accordingly.

[mikemhenry]

Ah I misunderstood the issue, @ijpulidos yes feel free to just setup some logic to skip for this test (I thought that work around would affect the get_available_force_fields machinery) and we could clean up the logic of get_available_force_fields in a different PR

[ijpulidos]

This one is superseded by #290 , I'm going to have this for testing purposes for now.

[mikemhenry]

Superseded by #290