Skip to content

Replaced std::random_shuffle with std::shuffle in tri #24062

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.

Sign up for GitHub

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

Merged
tacaswell merged 14 commits into from
Oct 28, 2022
Merged

Replaced std::random_shuffle with std::shuffle in tri #24062

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
601d92a
removed RandomNumberGenerator class, included <random>, replaced rand…
tybeller Sep 30, 2022
4bf6696
Merge branch 'matplotlib:main' into shuffleFix
tybeller Oct 2, 2022
1ae3610
Create 24062-tb.rst
tybeller Oct 2, 2022
c8e0531
Update doc/api/next_api_changes/behavior/24062-tb.rst
tybeller Oct 4, 2022
bd3dc8a
Update doc/api/next_api_changes/behavior/24062-tb.rst
tybeller Oct 5, 2022
4648dda
Update 24062-tb.rst
tybeller Oct 5, 2022
d77b191
Update 24062-tb.rst
tybeller Oct 6, 2022
0427513
Merge branch 'matplotlib:main' into shuffleFix
tybeller Oct 10, 2022
c4fb35e
Merge branch 'matplotlib:main' into shuffleFix
tybeller Oct 12, 2022
e096d5e
Update 24062-tb.rst
tybeller Oct 12, 2022
cf0613b
Merge branch 'matplotlib:main' into shuffleFix
tybeller Oct 14, 2022
d2d0d11
removed random number generator from _tri.h
tybeller Oct 14, 2022
a06bae9
Update _tri.cpp
tybeller Oct 14, 2022
3513a85
Update src/tri/_tri.h
oscargus Oct 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions doc/api/next_api_changes/behavior/24062-tb.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Change in TrapezoidMapTriFinder, triangulation interpolation, and refinement output due to change in shuffle.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

@oscargus oscargus Oct 4, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

would solve the failing doc build (if I got the length right...).

@QuLogic QuLogic Oct 4, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're having trouble aligning the underline, then that's a sign the title is probably too long. I would leave out all of the details and just say "Change in triangulation output ..." Also, no trailing period in titles.

With std::random_shuffle removed from C++17, TrapezoidMapTriFinder, triangular grid interpolation and refinement now use std::shuffle with a new random generator which may give different results.

@QuLogic QuLogic Oct 4, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Different results how? Does a plot change? Is the order different? This is too vague and as a user I have no idea whether I need to do anything here. Also, please word-wrap lines at 80 characters.

20 changes: 4 additions & 16 deletions src/tri/_tri.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

#include <algorithm>
#include <set>
#include <random>


TriEdge::TriEdge()
Expand Down Expand Up @@ -1465,8 +1466,8 @@ TrapezoidMapTriFinder::initialize()
_tree->assert_valid(false);

// Randomly shuffle all edges other than first 2.
RandomNumberGenerator rng(1234);
std::random_shuffle(_edges.begin()+2, _edges.end(), rng);
std::mt19937 rng(1234);
std::shuffle(_edges.begin()+2, _edges.end(), rng);

// Add edges, one at a time, to tree.
size_t nedges = _edges.size();
Expand Down Expand Up @@ -2055,17 +2056,4 @@ TrapezoidMapTriFinder::Trapezoid::set_upper_right(Trapezoid* upper_right_)
upper_right = upper_right_;
if (upper_right != 0)
upper_right->upper_left = this;
}



RandomNumberGenerator::RandomNumberGenerator(unsigned long seed)
: _m(21870), _a(1291), _c(4621), _seed(seed % _m)
{}

unsigned long
RandomNumberGenerator::operator()(unsigned long max_value)
{
_seed = (_seed*_a + _c) % _m;
return (_seed*max_value) / _m;
}
}

@oscargus oscargus Oct 14, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please end the file with an empty line. That will fix the pre-commit issue. (The pyside6 issue is fixed in #24158.)