-
Notifications
You must be signed in to change notification settings - Fork 77
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
Rework harmonic oscillator free energy tests for multistate samplers to run on travis #446
Conversation
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.
This is awesome, thank you!
Just for curiosity, what's the reason why you had to re-generate the cython code? Do you think this might be related to the mixing problems with multiple GPUs we observed (choderalab/yank#1130)?
That was accidental. It appears that installing the code locally caused that to happen.
Hm. It could be that the view into the arrays somehow skipped every other element. But didn't we only see this issue if we were using MPI? How would MPI be interacting with this? |
The cython changelog doesn't list any bugfixes that seem suspiciously like ours since 0.29.4. |
I think it's fine to track the .c file. It probably shortens slightly installation and, in principle, it allows us to not rely on the cython dependency.
Yes, you're probably right. I'm going to take another look at the problem in the next few days though. If this didn't change anything, maybe there was something that changed in mpich that fixed it. |
Description
This PR reworks the harmonic oscillator free energy tests for multistate samplers, testing them both with and without unsampled states and ensuring they are fast enough to run on travis (fixes #441).
I've also converted the
warnings.warn
messages about the experimental API tologger.warn
so they can be more easily suppressed with the logger level.