Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop Python 3.9, add Python 3.11 to CI testing #252

Closed
wants to merge 3 commits into from
Closed

Conversation

dotsdl
Copy link
Member

@dotsdl dotsdl commented Mar 2, 2024

Python 3.9 support ends on 2024.04.05 according to the NEP29 schedule.
Dropping it from CI and starting testing of Python 3.11.

@dotsdl
Copy link
Member Author

dotsdl commented Mar 2, 2024

Looks like we're still stuck in the water on Python 3.11. I think this may stem from ambertools still?

@mikemhenry, @j-wags, @mattwthompson is this consistent with your current understanding? It looks like CI for both openfe and openff-toolkit are both currently set up to run tests against Python 3.11, so it's unclear why it isn't working here.

@codecov-commenter
Copy link

codecov-commenter commented Mar 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.31%. Comparing base (7d80068) to head (2be5213).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #252      +/-   ##
==========================================
+ Coverage   81.86%   82.31%   +0.45%     
==========================================
  Files          26       26              
  Lines        3066     3065       -1     
==========================================
+ Hits         2510     2523      +13     
+ Misses        556      542      -14     

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

@mattwthompson
Copy link
Contributor

Most of the OpenFF stack works on 3.11 (3.12, even). You're right that the problem is AmberTools - if you can afford to bump from 22 to 23 this should open some doors ...

... but you're not pinning to AmberTools 22 (or explicitly including it at all) - the problem is in openmmforcefields: openmm/openmmforcefields#281

That's a hard constraint in packaging: https://prefix.dev/channels/conda-forge/packages/openmmforcefields

I can help steward openmmforcefields changes across the finish line - I am happy to help, review changes, etc. but I really need somebody from the OpenMM/Chodera lab sphere of influence to lead the charge

@dotsdl
Copy link
Member Author

dotsdl commented Mar 4, 2024

Ah cool, thanks @mattwthompson! Aren't openff-toolkit and openfe also dependent on openmmforcefields? If so, do we know why they are not seeing this resolution failure with openmmforcefields pinning to ambertools 22?

@mattwthompson
Copy link
Contributor

mattwthompson commented Mar 4, 2024

Aren't openff-toolkit and openfe also dependent on openmmforcefields?

In reverse order: yes and (surprisingly) no, although it's used in an example. openmmforcefields is in principle downstream of OpenFF, AmberTools is upstream of us all.

If so, do we know ...

Right you are! I was also confused by this. It actually seems like this should not be possible. (In short: AmberTools 23 is the only version that builds with Python 3.11, but openmmforcefields has not been updated+released with the relevant changes. OpenFF gets around this by not needing this particular combination of packages.) It took some digging around in the mud, but I think I found what's going on. Based on recent CI logs, OpenFE is just installing against an old build of an old version of openmmforcefields; when 0.11.2 was rebuilt with ambertools <23 the repodata was not patched (easy to forget to do, it's a hassle) so the old build still exists out in the wild.

image

Skimming the linked issue (281) above it appears the behavior differences are limited to some niche GAFF atom-typing corner cases I don't fully understand but seems non-trivial. Our (OpenF*) stacks should not be affected AFAICT - not sure how this all affects your coordination here.

If the only goal is to get Python 3.11 built with this dependency list, it should be doable with ambertools =22 and openmmforcefields =0.11.2 (or unconstrained). (Of course, patching and making new versions is the ideal, and more expensive, path forward.)

@IAlibay
Copy link
Member

IAlibay commented Mar 5, 2024

GAFF atom-typing corner cases I don't fully understand but seems non-trivial. Our (OpenF*) stacks should not be affected AFAICT - not sure how this all affects your coordination here.

:/ I think unfortunately this "technically" affects OpenFE. As in, some of our users are expecting to be able to use GAFF, so issues with using it will impact our users (thanks for looking into this @mattwthompson - it looks like we need to pin things in our recipes)

@dotsdl
Copy link
Member Author

dotsdl commented Aug 20, 2024

We were able to satisfy this in #283. Closing!

@dotsdl dotsdl closed this Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants