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
BUG/ENH: Allow bit generators to supply their own constructor #22014
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -2695,3 +2695,16 @@ def test_contig_req_out(dist, order, dtype): | |||||
assert variates is out | ||||||
variates = dist(out=out, dtype=dtype, size=out.shape) | ||||||
assert variates is out | ||||||
|
||||||
|
||||||
def test_generator_ctor_old_style_pickle(): | ||||||
rg = np.random.Generator(np.random.PCG64DXSM(0)) | ||||||
rg.standard_normal(1) | ||||||
# Directly call reduce which is used in pickline | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed, thanks. |
||||||
ctor, args, state_a = rg.__reduce__() | ||||||
# Simulate unpickling an old pickle that only has the name | ||||||
assert args[:1] == ("PCG64DXSM",) | ||||||
b = ctor(*args[:1]) | ||||||
b.bit_generator.state = state_a | ||||||
state_b = b.bit_generator.state | ||||||
assert state_a == state_b |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -2020,3 +2020,21 @@ def test_broadcast_size_error(): | |||||
random.binomial([1, 2], 0.3, size=(2, 1)) | ||||||
with pytest.raises(ValueError): | ||||||
random.binomial([1, 2], [0.3, 0.7], size=(2, 1)) | ||||||
|
||||||
|
||||||
def test_randomstate_ctor_old_style_pickle(): | ||||||
rs = np.random.RandomState(MT19937(0)) | ||||||
rs.standard_normal(1) | ||||||
# Directly call reduce which is used in pickline | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed, thanks. |
||||||
ctor, args, state_a = rs.__reduce__() | ||||||
# Simulate unpickling an old pickle that only has the name | ||||||
assert args[:1] == ("MT19937",) | ||||||
b = ctor(*args[:1]) | ||||||
b.set_state(state_a) | ||||||
state_b = b.get_state(legacy=False) | ||||||
|
||||||
assert_equal(state_a['bit_generator'], state_b['bit_generator']) | ||||||
assert_array_equal(state_a['state']['key'], state_b['state']['key']) | ||||||
assert_array_equal(state_a['state']['pos'], state_b['state']['pos']) | ||||||
assert_equal(state_a['has_gauss'], state_b['has_gauss']) | ||||||
assert_equal(state_a['gauss'], state_b['gauss']) |
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.
It would be nice if we could refactor this to avoid the circular import
_pickle
-> _generator/mtrand ->_pickle
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.
well, not really circular since
_generator
/mtrand
import_pickle
in the__reduce__
method, but the pattern is convoluted.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.
I think the only way to avoid would be to move these to be closed to the class definitions. There is always something circular required here since the main class needs its constructor, and the constructor needs the main class.
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.
That's very common in
__reduce__
implementations, IME.