You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix: simulating no longer turns on deterministic cuDNN - #2177
simulate_for_sbi and simulate_in_batches seed the random states with seed_all_backends, which also sets torch.backends.cudnn.deterministic = True and torch.backends.cudnn.benchmark = False. Those are global to the process, so one call to
a simulation helper switched the rest of the session to deterministic cuDNN algorithms and
could slow down later training of convolutional embedding nets.
This separates the two concerns. seed_random_states draws the seed and seeds random,
numpy, torch and torch.cuda. seed_all_backends is now that call plus the two cuDNN lines,
so its behavior is unchanged for the autouse fixture in tests/conftest.py and for the
test call sites. The simulation paths use seed_random_states.
Seeding itself does not change: with seed=None the seed is still drawn from PyTorch's
random state, so an upstream torch.manual_seed keeps reproducing a simulate_for_sbi(seed=None) call, and the per-batch seeds used when num_workers > 1 are
drawn from the same state as before.
The issue suggests an option of seed_all_backends. I used a second function instead: a
call like seed_all_backends(seed, deterministic_cudnn=False) reads as a contradiction,
and this way seed_all_backends keeps exactly its current meaning for the people who do
want bit-reproducible convolutions. Say the word if you would rather have the flag.
I did not export seed_random_states in sbi/utils/__init__.py, matching seed_all_backends, which is imported from sbi.utils.sbiutils by the tests that use
it. No public API is added or changed.
The two lines in seed_all_backends are the only place the package touches cuDNN, and no
notebook or page in docs/ mentions it, so there is nothing else to update.
User visible part: anybody who was relying on a simulate_for_sbi call to quietly make
their convolutional training deterministic now gets the torch default, and has to call seed_all_backends(seed) or set the flags themselves. That is the point of the issue, but
it is still a behavior change worth a line in the release notes.
How I checked it:
The repro from the issue (simulate_for_sbi(lambda t: t, BoxUniform(...), 5) then printing torch.backends.cudnn.deterministic) prints True on main and False here.
test_simulating_leaves_cudnn_flags fails on main for both seed=None and seed=42
and passes with the change. The fixture sets deterministic=False, benchmark=True before
the call on purpose: at the all-False default a test cannot tell "left alone" from
"set to False", so it would pass without the fix.
With num_workers > 1 the seeded simulator runs in a worker process, whose flags the
parent cannot see, so that call site is covered by the direct seed_random_states test
rather than by pretending the end-to-end test observes it.
Reproducibility checked locally: simulate_for_sbi at num_workers 1 and 2 and simulate_in_batches at num_workers=2 return identical x for seed=42 across two
calls, and seed=None is still reproduced by an upstream torch.manual_seed(0).
The three new tests take 0.02s each, so none is marked slow. pytest -n auto -m "not slow and not gpu" gives 7180 passed, 25 skipped, 116 xfailed in 5:31, and ruff, pyright sbi and pre-commit run --all-files pass.
I used Claude to explore the code, sanity-check the plan and draft this
patch and its tests, and verified the behavior locally as described above.
simulate_for_sbi and simulate_in_batches seeded through seed_all_backends, which
also sets torch.backends.cudnn.deterministic and clears benchmark. Both flags are
global to the process, so simulating switched convolutional training running later
in the same session onto deterministic cuDNN algorithms.
The seeding moves to seed_random_states in sbi/utils/sbiutils.py, and
seed_all_backends calls it before setting the two flags, which keeps its behavior
for the autouse fixture in tests/conftest.py and for existing callers. The
simulation paths, including the simulator_seeded closures used with num_workers >
1, call seed_random_states instead.
Seeding itself is unchanged: with seed=None the seed is still drawn from the torch
random state, so an upstream torch.manual_seed keeps reproducing a
simulate_for_sbi(seed=None) call, and the per-batch seeds are drawn from the same
state as before.
Fixessbi-dev#2159
No actionable comments were generated in the recent review.
Walkthrough
Random-state seeding is separated from cuDNN configuration. Simulation functions now seed random states without changing cuDNN flags. Tests cover both seeding helpers and simulation behavior.
seed_random_states seeds random states without changing cuDNN flags. seed_all_backends calls it and sets deterministic cuDNN flags. Tests verify both behaviors.
Simulation functions use seed_random_states before sampling or simulator calls. Tests check that simulation leaves cuDNN flags unchanged for omitted and supplied seeds.
Simulation helpers no longer change global cuDNN settings, and seed_all_backends keeps its previous behavior. Users who relied on simulation to enable deterministic convolutions must now set the flags themselves. No merge-blocking risk remains.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title names the main change: simulation helpers no longer enable deterministic cuDNN. The changeset separates seed_random_states from seed_all_backends, and the simulation paths now call the new f…
Linked Issues check
Issue #2159 requires that simulation helpers seed random states without changing the process-wide cuDNN flags, and that seed=None still reproduces under an upstream torch.manual_seed. At head, sbi/sim…
Out of Scope Changes check
All changes map to #2159. The sbi/utils/sbiutils.py change splits seed_all_backends into seed_random_states plus a cuDNN-setting wrapper, which is the requested separation. The call-site edits in sbi/…
Description check
The description states the problem (seed_all_backends sets process-global cuDNN flags during simulation), the fix (split seed_random_states from seed_all_backends), and the linked issue (Fixes #2159).…
Full details: Title check
Explanation
The title names the main change: simulation helpers no longer enable deterministic cuDNN. The changeset separates seed_random_states from seed_all_backends, and the simulation paths now call the new function, so the title matches the primary behavior fix. It uses the conventional fix: prefix.
Full details: Linked Issues check
Explanation
Issue #2159 requires that simulation helpers seed random states without changing the process-wide cuDNN flags, and that seed=None still reproduces under an upstream torch.manual_seed. At head, sbi/simulators/simutils.py (simulate_in_batches and its simulator_seeded closure) and sbi/utils/simulation_utils.py (simulate_for_sbi and its simulator_seeded closure) call seed_random_states, which seeds random, numpy, torch and torch.cuda only. seed_random_states draws the seed from torch.randint when seed is None, so the seed=None behavior is kept. seed_all_backends now delegates to seed_random_states and then sets cudnn.deterministic=True and cudnn.benchmark=False, so users who want deterministic cuDNN keep an explicit path. The issue also names simulate_from_theta; no such function appears in the sbi/ file listing at head, so that part of the scope is not present here. The added tests in tests/simulator_utils_test.py cover the cuDNN flag behavior for both seed=None and seed=42. The PR author reports the test and reproducibility results; I did not re-run them.
Full details: Out of Scope Changes check
Explanation
All changes map to #2159. The sbi/utils/sbiutils.py change splits seed_all_backends into seed_random_states plus a cuDNN-setting wrapper, which is the requested separation. The call-site edits in sbi/simulators/simutils.py and sbi/utils/simulation_utils.py switch the simulation paths to the new function. The test additions verify the requested behavior. No unrelated files or logic changes were identified in the summary or in the inspected hunks. Public API change is limited to the new seed_random_states name, which the PR author states is not exported from sbi/utils/init.py.
Full details: Description check
Explanation
The description states the problem (seed_all_backends sets process-global cuDNN flags during simulation), the fix (split seed_random_states from seed_all_backends), and the linked issue (Fixes #2159). It covers the template's Anything else section with design rationale, open coordination with #1738, the user-visible behavior change, and local validation. The checklist is filled in. The AI-assistance disclosure is present. Minor gap: the description does not restate the specific test-run detail as a checklist-linked item, but all template sections are substantively addressed.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: a03e836c-1f64-48c7-b137-edea128ce78e
Commits
Reviewing files that changed from the base of the PR and between 799e324 and 26ce8d2.
Files selected for processing: 4
sbi/simulators/simutils.py
sbi/utils/sbiutils.py
sbi/utils/simulation_utils.py
tests/simulator_utils_test.py
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.31%. Comparing base (799e324) to head (26ce8d2).
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
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.
What does this PR do?
simulate_for_sbiandsimulate_in_batchesseed the random states withseed_all_backends, which also setstorch.backends.cudnn.deterministic = Trueandtorch.backends.cudnn.benchmark = False. Those are global to the process, so one call toa simulation helper switched the rest of the session to deterministic cuDNN algorithms and
could slow down later training of convolutional embedding nets.
This separates the two concerns.
seed_random_statesdraws the seed and seedsrandom,numpy, torch and torch.cuda.
seed_all_backendsis now that call plus the two cuDNN lines,so its behavior is unchanged for the autouse fixture in
tests/conftest.pyand for thetest call sites. The simulation paths use
seed_random_states.Seeding itself does not change: with
seed=Nonethe seed is still drawn from PyTorch'srandom state, so an upstream
torch.manual_seedkeeps reproducing asimulate_for_sbi(seed=None)call, and the per-batch seeds used whennum_workers > 1aredrawn from the same state as before.
Does this close any issues?
Fixes #2159
Anything else we should know?
seed_all_backends. I used a second function instead: acall like
seed_all_backends(seed, deterministic_cudnn=False)reads as a contradiction,and this way
seed_all_backendskeeps exactly its current meaning for the people who dowant bit-reproducible convolutions. Say the word if you would rather have the flag.
simulator_seededclosures. That PR is still open, so this onetargets
main; I will rebase once feat: parallel simulation with simulate_from_theta and parallelize_simulator #1738 lands to keep the two in sync.seed_random_statesinsbi/utils/__init__.py, matchingseed_all_backends, which is imported fromsbi.utils.sbiutilsby the tests that useit. No public API is added or changed.
seed_all_backendsare the only place the package touches cuDNN, and nonotebook or page in
docs/mentions it, so there is nothing else to update.simulate_for_sbicall to quietly maketheir convolutional training deterministic now gets the torch default, and has to call
seed_all_backends(seed)or set the flags themselves. That is the point of the issue, butit is still a behavior change worth a line in the release notes.
How I checked it:
The repro from the issue (
simulate_for_sbi(lambda t: t, BoxUniform(...), 5)then printingtorch.backends.cudnn.deterministic) printsTrueonmainandFalsehere.test_simulating_leaves_cudnn_flagsfails onmainfor bothseed=Noneandseed=42and passes with the change. The fixture sets
deterministic=False, benchmark=Truebeforethe call on purpose: at the all-
Falsedefault a test cannot tell "left alone" from"set to False", so it would pass without the fix.
num_workers > 1the seeded simulator runs in a worker process, whose flags theparent cannot see, so that call site is covered by the direct
seed_random_statestestrather than by pretending the end-to-end test observes it.
simulate_for_sbiatnum_workers1 and 2 andsimulate_in_batchesatnum_workers=2return identicalxforseed=42across twocalls, and
seed=Noneis still reproduced by an upstreamtorch.manual_seed(0).slow.pytest -n auto -m "not slow and not gpu"gives 7180 passed, 25 skipped, 116 xfailed in 5:31, andruff,pyright sbiandpre-commit run --all-filespass.I used Claude to explore the code, sanity-check the plan and draft this
patch and its tests, and verified the behavior locally as described above.
Checklist
uv run pytest -n auto -m "not slow and not gpu"passes.uv run pre-commit run --all-filespasses (ruff and formatting).uv run pyright sbipasses.with
pytest.mark.slow.