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

Add s4 field to Xoshiro #51332

Merged
merged 5 commits into from
Sep 15, 2023
Merged

Add s4 field to Xoshiro #51332

merged 5 commits into from
Sep 15, 2023

Conversation

nhz2
Copy link
Contributor

@nhz2 nhz2 commented Sep 15, 2023

Fixes #51255.
Redo of #51271.
Instead of creating a new type of AbstractRNG, this PR adds an optional field to the existing Xoshiro struct.

#49110 added an additional state to the task-local RNG. However, before this PR copy(default_rng()) did not include this extra state, causing subtle errors in Test where copy(default_rng()) is assumed to contain the full task-local RNG state.

@brenhinkeller brenhinkeller added the domain:randomness Random number generation and the Random stdlib label Sep 15, 2023
@nhz2 nhz2 marked this pull request as ready for review September 15, 2023 14:23
@vtjnash vtjnash merged commit 41b41ab into JuliaLang:master Sep 15, 2023
4 of 6 checks passed
@rfourquet rfourquet added the backport 1.10 Change should be backported to the 1.10 release label Sep 17, 2023
rfourquet added a commit that referenced this pull request Sep 17, 2023
oscardssmith pushed a commit that referenced this pull request Sep 17, 2023
NHDaly pushed a commit that referenced this pull request Sep 20, 2023
This PR adds an optional field to the existing `Xoshiro` struct to be
able to faithfully copy the task-local RNG state.

Fixes #51255
Redo of #51271

Background context: #49110 added an additional state to the task-local
RNG. However, before this PR `copy(default_rng())` did not include this
extra state, causing subtle errors in `Test` where `copy(default_rng())`
is assumed to contain the full task-local RNG state.
NHDaly pushed a commit that referenced this pull request Sep 20, 2023
KristofferC pushed a commit that referenced this pull request Oct 3, 2023
This PR adds an optional field to the existing `Xoshiro` struct to be
able to faithfully copy the task-local RNG state.

Fixes #51255
Redo of #51271

Background context: #49110 added an additional state to the task-local
RNG. However, before this PR `copy(default_rng())` did not include this
extra state, causing subtle errors in `Test` where `copy(default_rng())`
is assumed to contain the full task-local RNG state.

(cherry picked from commit 41b41ab)
@KristofferC KristofferC mentioned this pull request Oct 3, 2023
31 tasks
KristofferC added a commit that referenced this pull request Nov 2, 2023
Backported PRs:
- [x] #50932 <!-- types: fix hash values of Vararg -->
- [x] #50975 <!-- Use rr-safe `nopl; rdtsc` sequence -->
- [x] #50989 <!-- fix incorrect results in `expm1(::Union{Float16,
Float32})` -->
- [x] #51284 <!-- Avoid infinite loop when doing SIGTRAP in arm64-apple
-->
- [x] #51332 <!-- Add s4 field to Xoshiro -->
- [x] #51397 <!-- call Pkg precompile hook in latest world -->
- [x] #51405 <!-- Remove fallback that assigns a module to inlined
frames. -->
- [x] #51491 <!-- Throw clearer ArgumentError for strip with two string
args -->
- [x] #51531 <!-- fix `_tryonce_download_from_cache` (busybox.exe
download error) -->
- [x] #51541 <!-- Fix string index error in tab completion code -->
- [x] #51530 <!-- Don't mark nonlocal symbols as hidden -->
- [x] #51557 <!-- Fix last startup & shutdown precompiles -->
- [x] #51512 <!-- avoid limiting Type{Any} to Type -->
- [x] #51595 <!-- reset `maxprobe` on `empty!` -->
- [x] #51582 <!-- Aggressive constprop in LinearAlgebra.wrap -->
- [x] #51592 <!-- correctly track element pointer in heap snapshot -->
- [x] #51326 <!-- complete false & true more generally as vals -->
- [x] #51376 <!-- make `hash(::Xoshiro)` compatible with `==` -->
- [x] #51557 <!-- Fix last startup & shutdown precompiles -->
- [x] #51845 
- [x] #51840 
- [x] #50663 <!-- Fix Expr(:loopinfo) codegen -->
- [x] #51863 <!-- LLVM 15.0.7-9 -->

Contains multiple commits, manual intervention needed:

- [ ] #51035 <!-- refactor GC scanning code to reflect jl_binding_t are
now first class -->
- [ ] #51092 <!-- inference: fix bad effects for recursion -->

Non-merged PRs with backport label:
- [ ] #51479 <!-- prevent code loading from lookin in the versioned
environment when building Julia -->
- [ ] #51414 <!-- improvements on GC scheduler shutdown -->
- [ ] #51366 <!-- Handle infix operators in REPL completion -->
- [ ] #50919 <!-- Code loading: do the "skipping mtime check for stdlib"
check regardless of the value of `ispath(f)` -->
- [ ] #50824 <!-- Add some aliasing warnings to docstrings for mutating
functions in Base -->
- [ ] #49805 <!-- Limit TimeType subtraction to AbstractDateTime -->
@KristofferC KristofferC removed the backport 1.10 Change should be backported to the 1.10 release label Nov 2, 2023
nalimilan pushed a commit that referenced this pull request Nov 5, 2023
This PR adds an optional field to the existing `Xoshiro` struct to be
able to faithfully copy the task-local RNG state.

Fixes #51255
Redo of #51271

Background context: #49110 added an additional state to the task-local
RNG. However, before this PR `copy(default_rng())` did not include this
extra state, causing subtle errors in `Test` where `copy(default_rng())`
is assumed to contain the full task-local RNG state.

(cherry picked from commit 41b41ab)
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.

Test: Child tasks have identical RNG streams after a testset in 1.10.0-beta2
5 participants