Conversation
This was referenced Aug 10, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes the GSEA permutation RNG seedable, so results can be reproduced.
make_permutationsseeds fromSmallRng::from_entropy(), so every GSEA run draws adifferent permutation set. Because the permutations are generated here rather than in R,
set.seed()in the calling R session cannot reach them, andWebGestaltRhas never beenable to offer reproducible GSEA. Two runs of one analysis can return a different number of
significant sets — measured at 26 rows vs 24, max ΔFDR 0.113 single-threaded and 0.691
across four threads. Requested in
WebGestaltR#9, open since 2021.
The change
GSEAConfiggainsseed: Option<u64>, defaulting toNone, threaded to the single callsite and into
make_permutations:SeedableRngwas already imported —from_entropyneeds it too.Backward compatible by construction.
Nonepreserves the current behaviour exactly, sonothing changes for callers that do not ask for a seed.
Why threading is not a problem
Permutations are generated serially up front (
gsea.rs:320) and shared across the parallelscoring that follows, so the RNG is never touched concurrently. Verified: seed 42 at
nThreads=1andnThreads=4give bit-identical p-values, FDRs and enrichment scores. OnlyNES moves, by 2–4e-16, which is floating-point summation in the normalisation step.
Verified
cargo test --allpasses, including themake_permutationsdoctest whose signature changed.End to end through the R package, on a branch of
WebGestaltRthat vendors this commit:two runs at
seed = 42produce bit-identicalp_val,fdrandES(compared attolerance = 0); a run atseed = 99differs inp_valandfdrwhileESis unchangedto every digit — the enrichment score has no randomness in it, so a seed that moved it would
mean the change reached further than intended.
Not in this PR
randstays at 0.8.5. Bump rand from 0.8.5 to 0.9.3 in the cargo group across 1 directory #29 bumps it to 0.9.3, whereSeedableRng::from_entropyis renamedto
from_os_rng(and removed again in 0.10). That rename lands on the same line this PRtouches, so whichever merges second needs a manual fix. Worth doing deliberately rather
than together with a correctness change.
fisher()is wrong in a way unrelated to this — see fisher() returns a chi-square density on the wrong degrees of freedom, inverting the p-value #30.