-
Notifications
You must be signed in to change notification settings - Fork 46
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
cosym fix np.random.seed #1621
cosym fix np.random.seed #1621
Conversation
Fix occassional test failure by also setting np.random.seed
Codecov Report
@@ Coverage Diff @@
## main #1621 +/- ##
=======================================
Coverage 66.60% 66.60%
=======================================
Files 614 614
Lines 68748 68750 +2
Branches 9571 9571
=======================================
+ Hits 45789 45791 +2
+ Misses 21029 21028 -1
- Partials 1930 1931 +1 |
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.
Tests pass for me on Windows 👍
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.
Just what was needed - this is (again) one of those tests which seems RNG dependent - thanks for the fix. To state obvious, it worked for me (but then it would, right?)
if seed is not None: | ||
flex.set_random_seed(seed) | ||
random.seed(seed) | ||
np.random.seed(seed) |
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.
👍
Yes - by definition the cosym procedure is dependent on a random number generator as it is used to generate random starting coordinates for the procedure: dials/algorithms/symmetry/cosym/__init__.py Line 295 in c9cdceb
In the command line script all the potentially relevant random seeds (random, flex.random, np.random) are set. Setting |
This meant that the post-scaling cos-angle clustering would vary between runs. See also dials/dials#1621
* Fix np.random.seed for xia2.multiplex This meant that the post-scaling cos-angle clustering would vary between runs. See also dials/dials#1621 * Use d_min cutoff from scaling for cluster analysis This probably gives more meaningful results than using all data to the edges of the data, and also makes the reported cluster completeness comparable to that reported after scaling. * Use proteinase_k fixture Gets the lists of experiment and reflection files, and removes any .refl files after a test has completed.
* Fix np.random.seed for xia2.multiplex This meant that the post-scaling cos-angle clustering would vary between runs. See also dials/dials#1621 * Use d_min cutoff from scaling for cluster analysis This probably gives more meaningful results than using all data to the edges of the data, and also makes the reported cluster completeness comparable to that reported after scaling. * Use proteinase_k fixture Gets the lists of experiment and reflection files, and removes any .refl files after a test has completed.
Fix occassional test failure by also setting np.random.seed