-
Notifications
You must be signed in to change notification settings - Fork 169
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
Use seed for all rng in blending to make a test run completely deterministic #450
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #450 +/- ##
==========================================
+ Coverage 84.17% 84.26% +0.09%
==========================================
Files 160 160
Lines 13058 13067 +9
==========================================
+ Hits 10991 11011 +20
+ Misses 2067 2056 -11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Nice addition, @mats-knmi!
@@ -274,7 +274,7 @@ def _get_error(scale): | |||
return shift, scale, R.reshape(shape) | |||
|
|||
|
|||
def resample_distributions(first_array, second_array, probability_first_array): | |||
def resample_distributions(first_array, second_array, probability_first_array, randgen): |
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.
Good to add randgen
also in the docstrings below.
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.
Ah yes indeed, I could also put a default value there to preserve backwards compatibility. I think that might be nicer.
In some places in the blending code the rng was done without using the provided seed. This meant that a run with a specified seed did not have a completely deterministic outcome and this makes testing harder. This PR fixes this.
See also: #449