Seed the Optuna sampler: reproducible sweeps, and fix the flaky falsifier test - #370
Merged
Conversation
run_optuna_sweep called optuna.create_study() with no sampler, so Optuna
drew a fresh random seed on every run. Which trials got explored, and
therefore the sweep's results, varied between identical invocations.
sample_param_combos() already takes seed=0 for exactly this reason; the
Optuna path was the only sampling in the module left unseeded.
This also made test_optuna_falsifier flaky. It runs 3 trials over
alpha in {1.0, 3.0, 5.0}, and the mock runner only fails for alpha > 2.0.
Optuna's startup trials are drawn uniformly with replacement, so roughly
one run in 27 drew alpha=1.0 three times, nothing was falsified, all
3 x 4 = 12 records accumulated, and "assert len(records) < 3 * len(seeds)"
failed. Measured over 300 unseeded runs: 11 failures (3.7%, against a
predicted (1/3)^3 = 3.7%), every one of them on the all-alpha=1.0 draw
and no failure on any other draw. The same 300 runs after seeding: zero
failures. It had reddened CI on unrelated pull requests since at least
July 9.
seed defaults to 0 and accepts None to opt back into Optuna picking its
own, which the new tests cover in both directions.
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.
Summary
run_optuna_sweepcalledoptuna.create_study(direction=direction)with no sampler, so Optuna drew a fresh random seed on every run. Which trials got explored, and therefore the sweep's results, varied between identical invocations.sample_param_combos()in the same module already takesseed=0for exactly this reason; the Optuna path was the only sampling left unseeded.The same gap made
test_optuna_falsifierflaky, which has been reddening CI on unrelated pull requests since at least July 9. It reappeared today on main ate3bc7b8.Why it failed
The test runs 3 trials over
alphain {1.0, 3.0, 5.0}, and the mock runner only fails whenalpha > 2.0. Optuna's startup trials are drawn uniformly with replacement, so roughly one run in 27 drawsalpha=1.0three times. Nothing is falsified, every trial runs all 4 seeds, 12 records accumulate, andassert len(records) < 3 * len(seeds)fails on12 < 12.Evidence
The failure rate is too low to see by running the test a few times, so I ran the sweep 300 times in-process and recorded which
alphavalues each run drew.(1.0, 1.0, 1.0)(3.0, 5.0, 5.0)3.7% measured against 3.7% predicted from
(1/3)^3, and the correlation is exact:(1.0, 1.0, 1.0)came up 11 times and failed all 11, while no other draw failed once.The fix
run_optuna_sweeptakesseed: int | None = 0and passes it toTPESampler(seed=seed). Deterministic by default, matchingsample_param_combos. PassingNoneopts back into Optuna choosing its own seed.Note this changes behaviour beyond the test: sweeps are now reproducible by default. For a benchmarking harness that seems like the right default, but it is a change worth a look.
Tests
Two new tests cover both directions: the same seed explores the same trials, and
seed=Nonedoes not. I checked thattest_optuna_sweep_is_reproduciblefails against the pre-fix code, so it actually guards the bug rather than passing either way.The previously-flaky test now passes 40 consecutive runs. Full fast suite: 492 passed, 4 skipped, up exactly 2 from the 490 baseline.
ruff check srcclean.The comment in
test_optuna_falsifierdescribed a draw the seeded sampler never produces, so it is updated to match what actually runs.Black wants to reformat two
with Live(...)blocks insweep.py. That is pre-existing drift unrelated to this change, so I left it alone rather than bundling it in.