Skip to content

feat: migrate multimapper-tiebreak RNG to in-tree splitmix64#118

Open
BenjaminDEMAILLE wants to merge 2 commits into
scverse:mainfrom
BenjaminDEMAILLE:bd/rng-splitmix64
Open

feat: migrate multimapper-tiebreak RNG to in-tree splitmix64#118
BenjaminDEMAILLE wants to merge 2 commits into
scverse:mainfrom
BenjaminDEMAILLE:bd/rng-splitmix64

Conversation

@BenjaminDEMAILLE

Copy link
Copy Markdown
Contributor

Summary

  • Replace the rand crate (StdRng) with STAR-rs's splitmix64-based deterministic generator for the two RNG consumers: shuffle_tied_prefix (SE/PE --outMultimapperOrder Random tie-break) and pick_primary_and_mapq (transcriptome-space primary pick).
  • Per docs-old/dev/porting-from-star-rs.md's RNG decision: this must land before STARsolo, whose UMI dedup depends on deterministic read order.
  • Keeps the existing per-read seed (run_rng_seed + read-name hash, already thread-invariant) — only the underlying generator changes, matching STAR-rs's own choice of an in-tree splitmix64 over a general-purpose PRNG crate (STAR-rs's DIVERGENCES.md D16: STAR's own mt19937 output is itself thread-count-dependent, so no external crate's output is any more "correct" to match).
  • Also migrates the test-only packed_stream.rs fuzz generator off rand, so the dependency drops from Cargo.toml entirely.

Test plan

  • cargo build --all-targets / test / clippy -D warnings / fmt --check all green (455 tests)

Replace the `rand` crate (StdRng) with STAR-rs's splitmix64-based
deterministic generator for the two RNG consumers ported so far:
`shuffle_tied_prefix` (SE/PE `--outMultimapperOrder Random` tie-break)
and `pick_primary_and_mapq` (transcriptome-space primary pick).

Per docs-old/dev/porting-from-star-rs.md's RNG decision: this must land
before STARsolo, whose UMI dedup depends on deterministic read order.
Keeps the existing per-read seed (run_rng_seed + read-name hash, already
thread-invariant); only the underlying generator changes, matching
STAR-rs's own choice of an in-tree splitmix64 over a general-purpose PRNG
crate (documented in STAR-rs's DIVERGENCES.md D16: STAR's own mt19937
output is itself thread-count-dependent, so no external crate's output is
any more "correct" to match).

Also migrates the test-only packed_stream.rs fuzz generator off `rand`,
so the dependency can be dropped from Cargo.toml entirely.

Gate green: cargo build --all-targets, clippy -D warnings, fmt, 455 tests.
@Psy-Fer

Psy-Fer commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

This is worth adopting early, as it should clean up the comparisons/tests quite a lot.

We can later assess if we continue to use this method at a later date.

@Psy-Fer

Psy-Fer commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

If you keep all your PRs on the same base commit, I can take care of merging them all. So no need to update each one.

@BenjaminDEMAILLE

Copy link
Copy Markdown
Contributor Author

If you keep all your PRs on the same base commit, I can take care of merging them all. So no need to update each one.

Oh fine so. will let you do the merge :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants