Skip to content

fix: make generate_session_id independent of the global random seed - #321

Open
pcbeingused333 wants to merge 1 commit into
pyfenn:mainfrom
pcbeingused333:fix/session-id-seed-independence
Open

fix: make generate_session_id independent of the global random seed#321
pcbeingused333 wants to merge 1 commit into
pyfenn:mainfrom
pcbeingused333:fix/session-id-seed-independence

Conversation

@pcbeingused333

Copy link
Copy Markdown
Contributor

Problem

generate_session_id() picks its adjective and noun with random.choice:

adj = random.choice(adjectives)
noun = random.choice(nouns)

set_seed() reseeds Python's global random state. So any run that calls set_seed(N) and then generate_session_id() gets the same word pair every time — only the secrets.token_hex(2) suffix varies:

set_seed(1234); generate_session_id()  # 20260908_1607_frosty_morning_07da
set_seed(1234); generate_session_id()  # 20260908_1607_frosty_morning_8e1d

Since set_seed() is the normal way to use this reproducibility-focused framework, session ids collapse to near-identical strings across runs, which defeats the point of a human-readable per-run identifier.

Fix

Use secrets.choice for the words too (secrets is already imported for the hex suffix). set_seed() still reseeds random/numpy/torch exactly as before — only the session-id word choice is moved off the seeded RNG.

Tests

Added test_word_choice_is_independent_of_the_global_seed: seeding with a fixed value and calling generate_session_id() repeatedly still yields varying word parts. It fails on main (frosty_morning pinned by the seed) and passes with the change. pytest tests/unit/test_reproducibility.py is green (19 passed); ruff check / ruff format clean.

Note: the pre-commit check's ty hook is red on main too (commit 0d1862e, on fenn/experimental/vision/normalize.py and fenn/logging.py), unrelated to this change.

`generate_session_id()` picked its adjective/noun with `random.choice`.
`set_seed()` reseeds Python's global `random`, so every run started with
the same seed produced the same word pair (only the `secrets.token_hex`
suffix varied), which defeats the point of a per-run identifier.

Use `secrets.choice` for the words too.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GtYgwYNfJ3wHrw9xrooVoB
@pcbeingused333

Copy link
Copy Markdown
Contributor Author

The pre-commit check is red on the ty hook, but that's pre-existing on main (commit 0d1862e, current HEAD, fails the same job on fenn/experimental/vision/normalize.py and fenn/logging.py — neither touched here). ruff check / ruff format pass; the four test jobs are the relevant ones and they're green.

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