-
Notifications
You must be signed in to change notification settings - Fork 81
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
[DNM] added openmm 8 beta to test matrix + fix bug with test #628
base: main
Are you sure you want to change the base?
Conversation
Interesting, getting the failure on this test: The output is not that helpful, will have to check this locally:
|
pytest gives me a better error:
|
This is the test
|
Spot in the openmm throwing the error https://github.com/openmm/openmm/blob/9d9ffb016b94fefe2f6aba5468173cf20ebb7433/openmmapi/src/ContextImpl.cpp#L193 |
Added in this PR openmm/openmm#3769 |
So I think the issue is more with the test than a behavior change in openmm. My theory is that |
@ijpulidos Do you mind taking a peak at this? |
.github/workflows/CI.yml
Outdated
- name: Install OpenMM nightly | ||
shell: bash -l {0} | ||
if: matrix.cfg.openmm == 'beta' | ||
run: | | ||
conda install -c conda-forge/label/openmm_rc -c conda-forge openmm | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there nightlies pushed to conda-forge
? Or only betas/RCs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mattwthompson So the "nightly" builds we do for openmm live at omnia-dev, but it doesn't use the conda-build
infrastructure so it is kind of problematic. What I did was create a new label conda-forge/label/openmm_dev
where I create new builds on demand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I did make a mistake on the name of this step!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks!
I will use this PR to fix the one bug we seem to have with a test |
Description
This PR adds openmm 8 to the testing matrix
Todos
Status
Changelog message
Resolves #626