-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
c++17 removed random_shuffle #24010
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
Comments
Would anyone mind if I worked on this? This would be my first contribution. From what I'm seeing it looks like it only needs to be replaced with the shuffle function, is that correct? |
@tybeller thank you I would certainly appreciate that. I think the issue is exactly as you say. One concern is I'm not sure what the policy is for supporting old versions of c++ is, so there is a chance you'd need a precompiler conditional to use |
Sounds good, right now im looking at using the standard shuffle using a mersenne twister engine to serve as the URBG with a random device generating the seed, I see before the RandomNumberGenerator class took a seed 1234 and I was wondering if I should use that instead or if I could possibly have RandimNumberGenerator output a URBG instead. |
My guess is that it doesn't matter that much how you do it. I think the shuffle is just supposed to make sure that the edges aren't in some bad order that would make the next algorithm perform really poorly. But I don't understand the domain logic very well so we'd need to hear from the matplotlib maintainers. |
It is absolutely fine to replace The shuffling needs to be deterministic hence the seed. If you create two That is probably too much information! So, summary of what this involves:
I am not classifying this as an API change. But because it may change the rending of a pixel or two in a typical output it could be so classified. If so, it would also require an API change note. |
Thanks tybeller and others!
…On Fri, Oct 28, 2022, 07:04 Thomas A Caswell ***@***.***> wrote:
Closed #24010 <#24010> as
completed via #24062 <#24062>
.
—
Reply to this email directly, view it on GitHub
<#24010 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACCVWKS34CWDZHWX3BB6DO3WFPMPZANCNFSM6AAAAAAQVKDGOE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
std::random_shuffle
is used here:https://github.com/matplotlib/matplotlib/blob/main/src/tri/_tri.cpp#L1469
It has been removed from c++17:
https://en.cppreference.com/w/cpp/algorithm/random_shuffle
The text was updated successfully, but these errors were encountered: