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

RFC: Move RandomDevice to Base.CoreRandom #35894

Closed
wants to merge 1 commit into from
Closed

Conversation

tkf
Copy link
Member

@tkf tkf commented May 15, 2020

In a recent conversation with @c42f (#35833 (comment), #35833 (comment)) it was brought up that Base has no reliable way to generate random numbers that cannot be accidentally interfered by the user. We used to(?) use Currently, we use (edit) Libc.rand for this purpose but it is not reliable as the user or other external library components may call srand.

It's ideal to have a more reliable way to generate "true" random numbers. For example, I proposed to use uuid4 in #35833 as an underlying identifier of the local "context variable." Using Libc.rand may be enough but RNG that has no seeding is ideal to ensure the correctness of the program.

To solve this problem, this PR moves RandomDevice from Random to Base.CoreRandom and exposes "mini"-Random interface inside Base.

@tkf tkf mentioned this pull request May 15, 2020
@c42f
Copy link
Member

c42f commented May 15, 2020

I agree this is a good idea, it seems important to have real randomness available in Base if we're serious about reliability of unique IDs in distributed systems.

Along with #35872 we'll be able to move uuid4 into base/uuid.jl without also pulling in all the complexity of the Random stdlib.

@rfourquet what do you think?

@JeffBezanson
Copy link
Member

Only tangentially related, but I'm not sure Base uses Libc.rand anymore --- where are we using it?

@tkf
Copy link
Member Author

tkf commented May 15, 2020

Ah, I trusted the comment in the code but I can't find any use by greping Base (only called in the tests?).

@rfourquet
Copy link
Member

@rfourquet what do you think?

I don't have a strong opinion on this. Libc.rand was an attempt to help Base get more independant from Random, but RandomDevice() might be a better candidate. Note also that while Libc.rand is not used anymore, RandomDevice() is morally used in the function _rand_string() in file.jl (on Windows). I'm not really sure why it has too though, and why a regular rand call wouldn't be enough.

Also, until we really become serious about making Base independant of Random, maybe a simpler approach here would be define just the RandomDevice struct in Base and rely on the rand methods defined in Random.

A last remark is concerning #34852 (which implements a task-local RNG). If this was about to land in the medium term, this might be another candidate for being the "rand provider of Base".

@rfourquet rfourquet added the randomness Random number generation and the Random stdlib label May 15, 2020
@KristofferC
Copy link
Member

Tangentially related but is there a way that this can help with making Random no longer type pirate the rand function (which is defined in Base).

@rfourquet
Copy link
Member

rfourquet commented May 15, 2020

is there a way that this can help with making Random no longer type pirate the rand function (which is defined in Base).

I don't think so, as we still probably want rand(Float64) to use the default RNG (which here remains defined in Random) and not RandomDevice().
Or maybe you meant with the task-local RNG PR? then maybe yes.

On this topic, I thought recently that it might have been cleaner to have rand belong in Random, and InteractiveUtils re-export it (which means packages would have to have using Random more often, but that feels OK). Material for 2.0...

@tkf
Copy link
Member Author

tkf commented May 15, 2020

It looks like the only place rand is actually used is ExponentialBackOff?

julia/base/error.jl

Lines 252 to 258 in 8f512f3

function iterate(ebo::ExponentialBackOff, state= (ebo.n, min(ebo.first_delay, ebo.max_delay)))
state[1] < 1 && return nothing
next_n = state[1]-1
curr_delay = state[2]
next_delay = min(ebo.max_delay, state[2] * ebo.factor * (1.0 - ebo.jitter + (rand(Float64) * 2.0 * ebo.jitter)))
(curr_delay, (next_n, next_delay))
end

I think Libc.rand is more appropriate here than RandomDevice.

@rfourquet
Copy link
Member

I think this can be closed, as we now have Libc.getrandom!, which is used to implement RandomDevice.

@rfourquet rfourquet closed this Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
randomness Random number generation and the Random stdlib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants