Skip to content
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

more methods working with a non-global RNG #8380

Closed
wants to merge 3 commits into from

Conversation

rfourquet
Copy link
Member

This is branched off of my other PR #8331 and continues it: the new commit here makes almost all rand methods work with a passed RNG. The main exception is rand(r::Range, ...) as I already touched this part of the code in #8309, so I prefer to wait before implementing it.
Also the value of (srand(0); rand(Uint64)) changes here, to make the function more natural and similar to rand(Uint128). I can revert this if considered a (worthless) breaking change.

as_unsigned(::Type{Int64} ) = Uint64
as_unsigned(::Type{Int128}) = Uint128

as_signed{T<:Integer128}(x::T) = convert(as_signed(T), x)
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Doesn't convert(Signed, x) already do this?

Copy link
Member

Choose a reason for hiding this comment

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

Then you could just do Signed(x) and Unsigned(x)...

Copy link
Member Author

Choose a reason for hiding this comment

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

It's weird, I remember having answered ("no") to Jeff's above comment but can't see it: can an inline comment be deleted when rebasing?

Yes this PR needs to be updated and rebased, unless it is deemed worthless.

@JeffBezanson
Copy link
Sponsor Member

Nice to have this refactored. Sorry I stepped on your changes; rebasing should be pretty easy though.

@JeffBezanson
Copy link
Sponsor Member

Should #8331 be closed in favor of this? This PR seems more comprehensive.

@rfourquet
Copy link
Member Author

Ok I will rebase tomorrow. Yes #8331 can be closed (there is 1 new commit in this current PR).

MersenneTwister's constructor making an instance with an Uint32 seed
were removed in commit 4fb4622. But a subsequent call to
srand(r::MersenneTwister, seed) would only accept an Uint32 as a seed,
and would always make r.seed an Uint32; this was not consistent.
The srand methods working on a MersenneTwister instance did not catch
up with those working on the global RNG. They are now made similar
thanks to the make_seed function, which implements the different
logics.
Also, the manual is more precise on the types of valid seeds.
And similarly for rand!, randbool.
Fixes JuliaLang#8360.
@rfourquet
Copy link
Member Author

Rebased and closed #8331.

@staticfloat
Copy link
Sponsor Member

Bump. This should be looked at in light of #8786

@rfourquet
Copy link
Member Author

Superseded by #8854.

@rfourquet rfourquet closed this Oct 30, 2014
@rfourquet rfourquet deleted the random-rng branch October 30, 2014 11:23
@ViralBShah ViralBShah added the domain:randomness Random number generation and the Random stdlib label Nov 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:randomness Random number generation and the Random stdlib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants