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
Changing std::rand() to a better inbuilt PRNG generator. #3239
Conversation
@Anushi1998 it looks like some tests are now failing. If you click on pycicle daint-3239... details and then the red 3 under Test/Fail you can see which ones failed. In this case it might be that the range of |
@msimberg |
Looking at one of the tests the problem seems to be that You should also restrict the range of the distribution to make sure that the elements don't wrap around to negative numbers when using Let me know if something is unclear! |
@msimberg |
Oh, even better. Thanks for correcting me! You seem to have gotten the hang of it. You might also find |
Yeah, Sure! I was actually checking for any way to pass w in |
@@ -21,7 +23,7 @@ | |||
//////////////////////////////////////////////////////////////////////////// | |||
unsigned int seed = std::random_device{}(); | |||
std::mt19937 gen(seed); | |||
std::uniform_int_distribution<> dis(0,std::numeric_limits<int>::max()); | |||
std::uniform_int_distribution<> dis(0,boost::integer_traits<int>::max()); |
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.
Why did you go away from std::numeric_limits
?
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.
Because Circle CI was failing violation of Boost min/max guidelines
.Should I do something else?
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.
Yah, sure. If you look around you'll find this: http://www.boost.org/doc/libs/1_34_0/more/lib_guide.htm (amongst other things). Essentially the Boost guidelines say that in order to avoid min
or max
being expanded as a macro you need to accordingly protect all of their uses. In case of std::numeric_limits<T>::max()
you would have to write (std::numeric_limits<T>::max)()
instead.
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.
Thanks for correcting me :)
I will fix it right away.
@hkaiser I think the pycicle daint-3239-gcc-6.2.0 Test is unrelated because I haven't changed those 3 files.It can be removed probably by restarting tests.What's your opinion? |
@Anushi1998, yeah, those are unrelated to your changes. |
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.
LGTM, thanks!
Thanks @Anushi1998! |
Fixes #2954
This PR is in continuation of PR #3204.