-
Notifications
You must be signed in to change notification settings - Fork 75
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
[WIP] Fix CI by upgrading to new GROMACS #301
base: master
Are you sure you want to change the base?
Conversation
|
Info: conda builds of the new GROMACS aren't passing yet, and I admit I haven't looked into why just yet |
(notes to self) The new GROMACS packages seem to have the tests involving The psuedovacuum calcs might be happy. I'll need to look closer at these to verify. Currently the only failing test in CI is I have two gromacs=2024 forcebalance dev conda envs on my computer.
I also have two directories:
When run locally, the water study passes/fails as follows, when run using the env in the column, in the forcebalance directory in the row:
I think this means that there's something special that I did in the fb-dev env a few months back that got the test passing, since the test results don't change depending on the folder I run them in. In recent commits, I've tried to get the env in CI matching the versions of deps in my local fb-dev env, to no avail. So, there's something causing tests to pass locally but fail in CI. It's not in some local changes to the test files since the fb-dev env passes tests in a clean checkout of this branch. I'm ruling out differences in the conda envs. Remaining possibilities are:
|
(more notes to self) All tests pass for me locally in the fb-dev env, (except the TINKER ones, I don't have that installed locally) The single non-passing test (AFAICT) both in GH CI and in my
What's confusing is that this DOES pass in my "good" local fb-dev environment.I can't figure out what's different between my local conda envs (
Even pip-installing the package from the (good) forcebalance directory to my fb-dev-clean conda env fails to get the water tutorial passing in the forcebalance dir. The fb-dev env had openmm 7.7 for some reason, though updating it to 8 made no difference. This is a little confusing because there's GH CI showed an AMOEBA test failing with the wrong version of openmm (needs a particular method signature that changed in OMM 8), but I think I didn't get this error because the AMOEBA tests don't run for me locally (because I don't have TINKER installed locally). |
I tried installing some specific versions of deps to The env diff between Collapsed env diff
|
My major finding in the past day has been that fresh conda envs using the current yaml pass all tests on mac (both intel and ARM hardware) with osx-64 packages. This is true both of envs built using my regular mamba (mamba 1.5.1 with conda 23.1.0) and micromamba (mamba 1.5.7). However ARM-native macs using osx-arm64 packages fail the water tutorial test, by having a seemingly slight numerical difference in the results. Linux tests are failing. My last commit tried switching the ubuntu tests to an older runner, but to no avail. Next steps are to debug what's going on with the failing codecov step, and to see if we can get the ubuntu tests happy again. |
Hi @leeping, (you can ignore everything above and the current messy diff for the purpose of this question, I'll clean it up before asking for a code review. This message is self-contained so all relevant info is here) I've got all but one test working with the following two changes:
The one test that doesn't pass is
I've tried downpinning versions of several math-y conda packages to determine which one is responsible for the numerical change, however I haven't found a culprit. Ultimately, the only signal I found is that it's due to slightly different numerical results on different architectures.
Basically, the packages built for intel 64-bit always pass, even when they're run on ARM machines emulating intel-64 using rosetta. The packages built for ARM machines always fail by slightly exceeding the absolute threshold. (The failures do appear to be consistent over a few dozen runs of various configurations - for the same machine configuration the results either do not vary at all, or vary only slightly, but never enough to change a pass to a fail or vice versa) Given the gradual and slight drift, my top theory right now is that this is due to something beyond our control on the computer architecture or OS level, so I'd advocate updating |
Hi Jeff, Thanks so much for your herculean efforts. I agree updating the value of EXPECTED_WATER_RESULTS should be okay.
|
Great, thanks @leeping! I'll update the expected results and clean up this PR, then I'll tag you again when it's ready for review. |
The bromine study seems to be a little random - it failed on ubuntu/3.8 5 runs ago but has passed since then, despite none of the commits touching that pathway or build steps.
|
…ptimization test tolerance
Hi @leeping - I've resolved the issue from my earlier message, and have things mostly passing. However I have a few more questions:
Let me know if you'd like to have a call to review this together. The current change set is pretty hacky and may be hard to read, but this is getting to the point where I know which functional changes to make but will need your input on where to put them. |
gmx dump
to come out in the GROMACS 2024.3 releasecutoff-scheme group
) is no longer available