-
Notifications
You must be signed in to change notification settings - Fork 70
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
Make the validation condition for random distributions lenient #550
Make the validation condition for random distributions lenient #550
Conversation
tests/integration/utils/random.py
Outdated
# method, we make the check lenient to avoid random | ||
# failures in the CI. (we still need the check to catch | ||
# the cases that are obviously wrong.) | ||
assert np.abs(theo_stdev - stdev) < stdev_tol * np.abs(theo_stdev) |
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.
The default stddev_tol = 2 = 200%
seems like a very high limit for a relative difference. Since standard deviation is always non-negative, what you are essentially checking here is that stddev < 3 * theo_stdev
. Is that your goal here, or is stddev_tol = 2
a typo?
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.
Oops. I spent much time on writing a comment and didn't do the job for the test itself ;) I revised the test to make it more sensible.
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.
Now the stddev check at the default tolerance is equivalent to:
theo_stdev/3 <= stddev <= 3*theo_stdev
If that's what you were going for, then feel free to merge.
…gate#550) * Make the validation condition for random distributions lenient * Fix typo * Catch too small standard variations against theoretical values as well * Replace unnecessary NumPy calls with Python primitives * Tighten the tolerance
The current validation check for random distributions is causing random test failures in the CI. It simply checks if the standard deviation of the samples is close enough to the theoretical value, but failing this test doesn't have any statistical significance unless we draw extremely many samples. (i.e., a particular run might be simply very unlucky.) until we find a better way to validate samples, this PR simply relaxes the check to avoid any unexpected and undesirable test failures.