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

Allow global setting of random number seed + better RepartitionedHybridTopologyFactory test #808

Draft
wants to merge 3 commits into
base: 0.10.x
Choose a base branch
from

Conversation

mikemhenry
Copy link
Contributor

Right now test_RepartitionedHybridTopologyFactory_energies fails sometimes depending on which amino is selected and the order that torsions are mapped. You can test this by running while CUDA_VISIBLE_DEVICES= pytest -k test_RepartitionedHybridTopologyFactory_energies; do echo "works"; done and eventually the test will fail. I looked at the output and traced the difference between a test passing and a test failing to atom index proposal order, which is random and we currently don't support setting the seed.

There also is some randomness from the amino that is selected to mutate. I think instead of mutating a random amino for testing, we should mutate all of them. I will check to see how much time this adds to the test.

Some ideas:

Set random number seed in a util file, and import that generator in tests that need it instead of doing rng = np.random.RandomState(42) everywhere

@codecov
Copy link

codecov bot commented Jun 17, 2021

Codecov Report

Merging #808 (26967ac) into 0.10.x (06bbd50) will decrease coverage by 2.66%.
The diff coverage is 100.00%.

@zhang-ivy
Copy link
Contributor

zhang-ivy commented Jun 18, 2021

Interesting.. so none of the other test functions in test_relative.py fail (e.g. test_HybridTopologyFactory_energies())? is it just test_RepartitionedHybridTopologyFactory()?
If none of the other test functions besides test_RepartitionedHybridTopologyFactory() are failing, then I don't think its a geometry engine random torsion problem, as the geometry engine is used in the other tests as well.

Also, test_RepartitionedHybridTopologyFactory() has 3 cases: alanine dipeptide in vacuum, alanine dipeptide in solvent, and an 8-mer peptide in solvent. Which one(s) are failing?

@jchodera jchodera added the tests Unit tests label Aug 30, 2021
@jchodera jchodera changed the title RNG Fix + better RepartitionedHybridTopologyFactory test Allow global setting of random number seed + better RepartitionedHybridTopologyFactory test Aug 30, 2021
@jchodera
Copy link
Member

May also help with #815

@mikemhenry mikemhenry changed the base branch from main to 0.10.x July 11, 2023 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants