Skip to content

fix(exp12): deterministic per-(seed,trial,bit) sampler and fixture path to restore sim tests#142

Merged
jverdicc merged 1 commit intomainfrom
codex/fix-failing-exp12_tests-in-sim-feature
Mar 2, 2026
Merged

fix(exp12): deterministic per-(seed,trial,bit) sampler and fixture path to restore sim tests#142
jverdicc merged 1 commit intomainfrom
codex/fix-failing-exp12_tests-in-sim-feature

Conversation

@jverdicc
Copy link
Owner

@jverdicc jverdicc commented Mar 2, 2026

Motivation

  • The exp12 simulation used a single shared sequential RNG stream across scenarios which made results scenario-order dependent and broke the monotonicity property test under topic budgeting.
  • The golden-fixture lookup in the test used a repository-relative path that could fail depending on the test working directory.

Description

  • Replace the shared LCG stream with a deterministic per-(seed, trial_idx, bit_idx) sampler implemented as uniform_u53(seed, trial_idx, bit_idx) and update binomial_sample to accept seed and trial_idx so scenario ordering is independent.
  • Remove the shared Lcg64 state and call binomial_sample(..., cfg.seed, trial_idx) for each trial.
  • Make the test robust by resolving the fixture via env!("CARGO_MANIFEST_DIR") when reading test_vectors/exp12_golden.json.
  • Update the golden fixture crates/discos-core/test_vectors/exp12_golden.json values to match the corrected deterministic output.

Testing

  • Ran cargo test -p discos-core --features sim --test exp12_tests and the test binary passed: both exp12_matches_golden_fixture and exp12_properties_hold succeeded.
  • Ran cargo test -p discos-core --features sim and the package tests completed successfully.
  • Attempted workspace checks: cargo fmt --check, cargo clippy --workspace --all-targets -- -D warnings, and cargo test --workspace were executed but surfaced pre-existing unrelated issues in other crates (formatting, lints, and CLI API/compile drift) and are therefore not blocking this exp12 fix.

Codex Task

@jverdicc jverdicc merged commit fa42970 into main Mar 2, 2026
2 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant