search

MolSSI / QCEngine

Quantum chemistry program executor and IO standardizer (QCSchema).
https://molssi.github.io/QCEngine/
BSD 3-Clause "New" or "Revised" License
163 stars 78 forks source link

Support OpenFF Toolkit v0.11+ in openmm.py #378

Closed Yoshanuikabundi closed 1 year ago

Yoshanuikabundi commented 1 year ago

Hi! This is my first contribution here, let me know if I've done anything wrong!

Description

The OpenFF Toolkit v0.11 includes breaking changes, including enforcing that conformer units are specified with the new OpenFF Units package rather than OpenMM units. This PR updates QCEngine to be compatible with the new version while preserving compatibility with the old versions.

Changelog description

Support OpenFF Toolkit v0.11+

Status

codecov[bot] commented 1 year ago

Codecov Report

Merging #378 (c219a8c) into master (2793f58) will decrease coverage by 18.55%. The diff coverage is 0.00%.

Additional details and impacted files
loriab commented 1 year ago

Thanks much for the update.

Most of the CI failures you see aren't your fault. If you edit these three characters (https://github.com/MolSSI/QCEngine/pull/376/commits/51f911914d04b7e086cb56a412887107a448454c) all but OpenMM will pass. In an unrelated PR, I still get some OpenMM problems: https://github.com/MolSSI/QCEngine/actions/runs/3220369981/jobs/5266984519 . Do you recognize those at all?

jthorton commented 1 year ago

In an unrelated PR, I still get some OpenMM problems:

Those should all be fixed by this PR, it looks like updating the CI has pulled in the version of the openff-toolkit which breaks qcengine.

loriab commented 1 year ago

In an unrelated PR, I still get some OpenMM problems:

Those should all be fixed by this PR, it looks like updating the CI has pulled in the version of the openff-toolkit which breaks qcengine.

Ok, good. Is the new toolkit released? Is there a particular version of the toolkit that needs to be disallowed? I'm good with the PR once it can pass CI.

Yoshanuikabundi commented 1 year ago

No worries!

If you edit these three characters (https://github.com/MolSSI/QCEngine/commit/51f911914d04b7e086cb56a412887107a448454c) all but OpenMM will pass.

Done. Looks like the new workflow needs approval. I expect this should pass CI but we'll see what happens!

In an unrelated PR, I still get some OpenMM problems:

Those should all be fixed by this PR, it looks like updating the CI has pulled in the version of the openff-toolkit which breaks qcengine.

Ok, good. Is the new toolkit released? Is there a particular version of the toolkit that needs to be disallowed? I'm good with the PR once it can pass CI.

Yep, Toolkit 0.11 was released about a month ago. Releases that don't include this PR should ideally be <0.11 but releases including this PR won't need this pin.

Yoshanuikabundi commented 1 year ago

A week or two is fine I think :)