chore(rust): bump rand 0.8 -> 0.10, rand_xoshiro 0.6 -> 0.8#391
chore(rust): bump rand 0.8 -> 0.10, rand_xoshiro 0.6 -> 0.8#391
Conversation
These two crates are coupled through rand_core and must move together; rand_xoshiro 0.8 requires the rand_core that ships with rand 0.9+. API churn handled in bootstrap.rs (rand 0.9 renamed the gen* family): - rng.gen::<bool>() -> rng.random::<bool>() - rng.gen::<f64>() -> rng.random::<f64>() - rng.gen_range(0..6) -> rng.random_range(0..6) Bit-identity preserved: Xoshiro256PlusPlus::seed_from_u64(seed) produces the same byte stream in 0.6 and 0.8, so existing bootstrap baselines remain valid. Verified locally on macOS Accelerate: - tests/test_rust_backend.py: 95 passed (includes Rust<->Python parity tests on bootstrap weights and bootstrap-derived SE/CI quantiles) - Bootstrap-focused sweep across the broader test suite: 372 passed - 467 total passing across the directly affected surfaces Supersedes #382 (rand) and #384 (rand_xoshiro). blas-src 0.10 -> 0.14 (#386) deferred pending an ndarray compatibility check; out of scope. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Overall Assessment
Executive Summary
Methodology
Code Quality
Performance
Maintainability
Tech Debt
Security
Documentation/Tests
Path to Approval
|
…pshot
Two fixes plus a CHANGELOG correction surfacing a finding from the
review work itself:
1. P1 - MSRV bump (rust/Cargo.toml):
rand_xoshiro 0.8 and rand 0.10 both declare rust-version = "1.85".
Cargo.toml previously advertised 1.84, which would silently break
anyone building from source on Rust 1.84 (CI uses stable so it
wouldn't be caught there). Bump to "1.85" as a deliberate MSRV
change, the cost of the upgrade.
2. P2 - bit-identity snapshot test (tests/test_rust_backend.py):
New TestRustBackend::test_bootstrap_weights_bit_identity_snapshot
pins fixed-seed (seed=42, n_bootstrap=2, n_units=4) bootstrap
weights for all three weight types. Existing tests verify only
shape, moments, and within-build seed-reproducibility, so a future
crate upgrade that preserves the distribution while changing the
draw stream would silently slip through. The snapshot test fails
loudly with a localized error message instead.
3. Honest CHANGELOG entry on the Webb byte shift (NEW finding from
side-by-side comparison against main):
The bit-identity claim in the original PR description was correct
for Rademacher and Mammen but WRONG for Webb. rand 0.9 reworked the
internal algorithm for random_range (improved rejection sampling),
so the same xoshiro byte stream produces different bucket selections
than the old gen_range did. Distributional properties unchanged
(still uniform over the 6-point support), aggregate inference
converges identically, but per-seed Webb draws are now different.
Verified against main on the same call:
Rademacher: identical
Mammen: identical
Webb: DIFFERENT (verified across both rows of the snapshot)
No internal test regression: 794 tests pass across all 9 webb-
mentioning test files with DIFF_DIFF_BACKEND=rust. The test suite
uses within-build seed-reproducibility, not cross-version baselines,
so the byte shift is invisible to existing tests. Documented in
CHANGELOG under [Unreleased] so users with external pinned Webb
baselines aren't surprised.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
/ai-review |
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment ✅ Looks good Executive Summary
Methodology
Code Quality
Performance
Maintainability
Tech Debt
Security
Documentation/Tests
|
…ay_equal Reviewer noted the snapshot test docstring claims "byte-for-byte / bit identity" but the assertion was assert_allclose(atol=1e-14, rtol=1e-14), which is slightly weaker than the wording suggests. Switch to np.testing.assert_array_equal so the assertion strength matches the stated contract. Snapshot values are either exact f64 (Rademacher = +/-1.0) or computed once via correctly-rounded IEEE 754 sqrt in Rust (Mammen's golden-ratio constants, Webb's sqrt(N/2) constants), so cross-platform bit-equality holds on conformant hardware. Inline comment added explaining the safety argument. If a runner ever fails this test on a hardware/sqrt drift basis (very unlikely on x86_64 / aarch64 / ARM64 Windows), we'd learn something useful about that runner before deciding to soften the check. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
/ai-review |
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment✅ Looks good Executive Summary
Methodology
Code Quality
Performance
Maintainability
Tech Debt
Security
Documentation/Tests
|
Summary
Bumps two coupled Rust crates that the bootstrap kernel depends on:
rand0.8 → 0.10rand_xoshiro0.6 → 0.8These move together because
rand_xoshiro 0.8requires therand_coreshipped byrand 0.9+. Bumping only one would fail to compile.Also bumps the crate's declared
rust-versionfrom1.84to1.85(both new dep versions require Rust 1.85+).API churn
rand 0.9renamed thegen*method family. Three call sites inrust/src/bootstrap.rs:rng.gen::<bool>()rng.random::<bool>()rng.gen::<f64>()rng.random::<f64>()rng.gen_range(0..6)rng.random_range(0..6)Bit-identity: per-weight-type breakdown
Verified by direct side-by-side comparison against
mainon the same(seed=42, n_bootstrap=2, n_units=4)call:mainWhy Webb shifted but the other two didn't:
random::<bool>()andrandom::<f64>()are byte-stable acrossrand 0.8→0.10.random_range(the integer range sampler used only by Webb) is not —rand 0.9reworked the internal algorithm to use improved rejection sampling. SoXoshiro256PlusPlus::seed_from_u64(seed)followed byrandom_range(0..6)consumes RNG bytes differently thangen_range(0..6)did.What this means:
n_bootstrapInternal validation (no regressions)
Local on macOS (
--features accelerate),DIFF_DIFF_BACKEND=rust:tests/test_rust_backend.py(full)pytest -k "bootstrap or wild"pytest -k "webb"test_path_sup_t_seed_reproducibility[webb])The test suite uses within-build seed-reproducibility (not cross-version baselines), so the Webb byte shift is invisible to existing tests. No baseline regeneration required.
New regression guard
Added
TestRustBackend::test_bootstrap_weights_bit_identity_snapshotwhich pins fixed-seed weights for all three weight types. Future RNG drift across crate versions will fail this test loudly with a localized error.CHANGELOG
Added an
[Unreleased]entry documenting the Webb byte shift so users with external pinned Webb baselines are not surprised on upgrade.Methodology references
N/A - dependency version bump with mechanical API rename. No identification, weighting, variance/SE family, or default behavior changed.
Validation
cargo build --features accelerate: cleanmaturin develop --release --features accelerate: cleanSecurity / privacy
No secrets/PII. Pure dependency upgrade.
Related
blas-src0.10 → 0.14 (Chore(deps): Update blas-src requirement from 0.10 to 0.14 in /rust #386) deferred pending an ndarray compatibility check; out of scope here.