Skip to content

Update tests to work without ambertools/with openeye#140

Merged
lilyminium merged 31 commits intomainfrom
remove-ambertools-dep
Mar 3, 2026
Merged

Update tests to work without ambertools/with openeye#140
lilyminium merged 31 commits intomainfrom
remove-ambertools-dep

Conversation

@lilyminium
Copy link
Contributor

@lilyminium lilyminium commented Feb 12, 2026

This PR updates tests to ensure they can run without ambertools, or with openeye, installed. Passing tests in [this set of actions](https://github.com/openforcefield/smee/pull/140/commits/08108785ce7fa7d4e0b4b87fc77114ceff51d5f50.

This also updates the CI workflow. I've changed the following:

  • creates an ambertools and openeye environment respectively
  • modified the workflow to not use the lockfile for Ubuntu environments. This was not possible for Mac because Pixi tries to resolve the cuda environments and runs into cross-platform issues, even though none of the environments here use the cu12 feature [?]. Pixi did successfully resolve the ambertools environment, though, so maybe I'm still doing something wrong.
  • The workflow was modified to not use the lockfile because for CI, it seems more desirable to test the most up-to-date versions of the ecosystem at any point, including very upstream dependencies that we don't specify in pyproject.toml.
  • It would be very nice to be able to test different Python versions in the GitHub matrix in the same way we're accustomed to doing with Conda workflows, instead of needing to clutter up pyproject.toml with additionally specified Pixi environments. I haven't worked out how to do this yet.

@lilyminium lilyminium marked this pull request as ready for review February 12, 2026 19:57
@lilyminium lilyminium marked this pull request as draft February 12, 2026 19:57
@lilyminium lilyminium changed the title Try testing without ambertools Update tests to work without ambertools/with openeye Feb 16, 2026
@lilyminium lilyminium self-assigned this Feb 16, 2026
@lilyminium lilyminium marked this pull request as ready for review February 17, 2026 09:46
@lilyminium
Copy link
Contributor Author

This should wait until #131 since that makes substantial changes to the CI / env workflow

fjclark added a commit to fjclark/smee that referenced this pull request Feb 17, 2026
@lilyminium lilyminium force-pushed the remove-ambertools-dep branch from 0810878 to 5c864c6 Compare February 20, 2026 04:51
Copy link
Contributor

@Yoshanuikabundi Yoshanuikabundi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good! Locking the environment on MacOS but not Ubuntu is weird, and it would be nice to fix it if we can, but I don't think it's necessarily blocking anything. I've found two possible candidates for what could be causing it, so would be good to investigate those before merging. I also note that Ambertools seems to be required in all environments - is that intended?

RE: Cluttering up pyproject.toml - I've really enjoyed keeping pixi stuff in pixi.toml, since it gets to be such a big file even in small projects and the more focused file makes things a lot easier to navigate. The downside is that it doesn't encourage you to use the dependency-groups and project.dependencies fields in standard pyproject.toml files, but we're not using them here anyway so that's not a huge loss.

I think the really big win we get from using Pixi environments in CI is that there's a single source of truth for environments that can be used by devs on their own machines, users trying the examples, and CI. It means potential contributors don't have to go digging in the CI config to figure out how to get started when we forget to document it properly, or the documentation goes out of date. I think this is a huge win that more than makes up for having to enumerate every environment in the matrix.

It would be nice to be able to construct ad-hoc environments from a list of features though! That might be the best of both worlds.

@j-wags j-wags assigned lilyminium and unassigned Yoshanuikabundi Feb 26, 2026
@lilyminium
Copy link
Contributor Author

Amazing -- thanks Josh, that update definitely did the trick! I really like the idea of a pixi.toml for separating things out from pyproject.toml -- will look into that.

@lilyminium
Copy link
Contributor Author

I do seem to have uncovered a real bug in interchange which is out of scope of this PR...

@lilyminium lilyminium merged commit f3d4c0b into main Mar 3, 2026
10 checks passed
@lilyminium lilyminium deleted the remove-ambertools-dep branch March 3, 2026 10:37
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.

2 participants