-
Notifications
You must be signed in to change notification settings - Fork 7.2k
Seed transform tests #6749
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
Seed transform tests #6749
Conversation
I don't mind reverting my changes. The reason why I didn't follow this approach is because the population of the |
True. And I need to look into how we can fix that. This is why I put back your changes to the sample inputs definition in 7d25068.
Yes, they will be different in shape. This is not desired and I will look into it as explained above. However, your PR also does not fix this as the setting of the seed only happens inside the test and thus at the same time as the fixture of this PR does. |
My original PR circumvents the problem by not having random calls for the specific problematic kernel during the I'm not sure I get how this PR fixes the issue as you say. Does Edit: Nevermind, I just realised you don't revert all the changes. You keep the non-randomness change on the |
Summary: * Revert "Add seeds on Kernel Info and reduce randomness for Gaussian Blur (#6741)" This reverts commit 6e72f2f. * add fixture to fix the RNG seed * re-add changes to gaussian_blur_* sample input shapes Reviewed By: NicolasHug Differential Revision: D40427476 fbshipit-source-id: b0aae476915f2d4e58146edf8e9339338bd78ad5 Co-authored-by: Vasilis Vryniotis <datumbox@users.noreply.github.com>
Follow-up to and partial revert of #6741.
Although it is ok to have
"random"
values in the inputs definition, we don't want actual random values inside our tests. The former communicates that the actual values don't really matter, while the latter makes sure we don't have flaky tests and can reproduce CI behavior locally.Thus, this PR reverts the seed changes of #6741 and adds a fixture that sets the RNG seed before every test. If #6750 is resolved, we can probably also put back the
"random"
sizes for thegaussian_blur_*
kernels.