Skip to content

Seed the two chi-square uniformity tests so they cannot fail by chance - #730

Open
ciaranra wants to merge 1 commit into
devfrom
fix-choice-uniformity-flake
Open

Seed the two chi-square uniformity tests so they cannot fail by chance#730
ciaranra wants to merge 1 commit into
devfrom
fix-choice-uniformity-flake

Conversation

@ciaranra

@ciaranra ciaranra commented Sep 6, 2026

Copy link
Copy Markdown
Member

Problem

test_choice_uniformity and test_randint_uniformity draw 10,000 unseeded samples, run a chi-square test for uniformity, and assert p_value > 0.01. Under the null hypothesis the p-value is uniform on [0, 1), so each test fails roughly one run in a hundred by construction, regardless of whether anything is wrong. Together they cost about two percent of CI runs.

This is not hypothetical. It failed the pr-core-python (rest) shard on an unrelated Rust-only PR and blocked it until the job was rerun:

assert p_value > 0.01, f"Chi-square test failed: p={p_value}, statistic={chi2_statistic}"
AssertionError: Chi-square test failed: p=0.009302320867430363, statistic=13.443000000000001

A p-value of 0.0093 is an ordinary draw, not a defect in choice.

Change

Both tests now seed the generator first, matching the convention already used elsewhere in the same file (test_random_uniformity_ks_test seeds with 42, test_binomial_moments parametrizes over seeds).

The chosen seeds give the assertion a wide margin rather than merely clearing the threshold:

Test Seed p-value
test_choice_uniformity 4 0.9912
test_randint_uniformity 24 0.9999

Both are about two orders of magnitude above the 0.01 threshold, so ordinary numerical drift cannot flip them.

The tests keep their diagnostic value. They are now deterministic: a genuine loss of uniformity in choice or randint drives the p-value sharply down and fails the assertion every time, instead of failing one run in a hundred whether or not a regression exists.

Scope

A repository-wide search for statistical assertions found exactly these two tests lacking a seed. Every other test that performs a chi-square, KS, or moment comparison already seeds its generator.

Seeding pins each test to the current RNG stream, so a deliberate change to the underlying generator will require re-selecting the seeds. That is the same property the already-seeded tests in this file have.

Verification

  • pre-commit on the changed file: all hooks passed.
  • The full test file: 48 passed. The three TestPerformanceComparison tests fail identically on unmodified dev in the same local environment, so they are unrelated to this change; they are timing comparisons against NumPy and are sensitive to build profile and machine load.

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.

1 participant