Skip to content

fix(data): seed load_dataset's random sampling with random_state - #593

Open
breimanntools wants to merge 1 commit into
masterfrom
feat/582-load-dataset-seed
Open

breimanntools wants to merge 1 commit into
masterfrom
feat/582-load-dataset-seed

Conversation

@breimanntools

Copy link
Copy Markdown
Owner

aa.load_dataset(random=True) was not reproducible: no random_state, and aa.options["random_state"] did not reach it. Three successive calls returned three different protein sets. Since load_dataset is the first line of most workflows, that nondeterminism propagated into everything downstream.

load_dataset(..., verbose=False, random_state=None)

Appended after verbose (the same position AAclust.__init__ uses), so every existing positional call keeps working.

No second mechanism: the Validate block gained one line, random_state = ut.check_random_state(random_state=random_state) — the same helper AAclust, CPP, dPULearn, TreeModel and ShapModel use — so options["random_state"] overrides an explicit argument exactly as documented elsewhere. Both directions are asserted.

The seed reaches the draw through one shared legacy RandomState, passed to both class draws: sharing one instance means the second class continues the stream rather than repeating it, and RandomState carries a cross-version stream guarantee, which is what makes "identical across processes" true rather than best-effort.

random=False is byte-identical — measured, not assumed

A pristine package was exported from the parent commit with git archive, then 83 digests (14 bundled datasets + Overview, 5–7 variants each: sha256 over to_csv, plus shape, plus per-column dtypes) were compared against the branch under PYTHONHASHSEED 0, 1, 2, 3, 42: 415/415 identical, 0 differences. The unseeded random=True path is unchanged too, checked with the global numpy state pinned.

No warning for unseeded random=True

Deliberate: no other stochastic entry point in the package warns on random_state=None, random=True is an explicit opt-in rather than a soft failure, and a warning would fire in the package's own notebooks and every docs build. The docstring carries the guidance instead.

Verification

18 new tests (26 collected) including a cross-process check via subprocess, a hypothesis sweep over seeds, the options-override precedence, and random=False unaffected by either. 768 tests pass. pyright: 4 pre-existing errors on this file, the same 4 on master — 0 new. Notebook re-executed with both random and random_state by name.

It uncovered a bigger problem — #588

While proving determinism, the same measurement showed load_dataset is not deterministic even with random=False: 30 of 83 digests change with PYTHONHASHSEED, because the class blocks are ordered by a set of string labels. And non_canonical_aa='gap' builds a regex character class from a set, so with - in play it can form a range — [U-X] silently replaces canonical V and W with gaps, [X-U] raises bad character range.

Both are filed as #588 (prio:1) rather than fixed here: the ordering fix changes committed AA_* notebook outputs, which is a deliberate output change needing its own re-execution pass.

Refs #582.

🤖 Generated with Claude Code

`aa.load_dataset(random=True)` had no seed, so the first line of most
workflows drew a different sample on every call and `options['random_state']`
never reached it: a benchmark, tutorial or bug report written with
`random=True` could not be reproduced by its own author.

`load_dataset` now takes `random_state` (appended after `verbose`, so every
positional call keeps working), resolved through `ut.check_random_state` like
AAclust, CPP and dPULearn, which also gives it the documented
`options['random_state']` override. The per-class draws share one
`numpy.random.RandomState` instance, so the classes continue one stream
instead of repeating it, and the legacy generator's permanent stream guarantee
makes a seed reproduce the same frame in another process or environment.

Nothing else changes. `random=False` keeps the deterministic head-of-class
selection, and an unseeded `random=True` keeps drawing from numpy's global
state: over all 14 bundled datasets (83 loading variants x 5 `PYTHONHASHSEED`
values, 415 comparisons) the sha256 of the returned frame is identical to a
pristine export of the parent commit, and with the global numpy state pinned
the unseeded random path matches byte for byte too.

An unseeded `random=True` deliberately stays silent: no other stochastic entry
point in the package warns on `random_state=None`, `random=True` is an explicit
opt-in to randomness, and a warning here would fire in the example notebook and
every docs build for a documented default.

Closes #582

Co-Authored-By: Claude Fable 5.1 <[email protected]>
@codecov

codecov Bot commented Sep 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.39%. Comparing base (fc8e21e) to head (e8d24b4).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #593   +/-   ##
=======================================
  Coverage   95.39%   95.39%           
=======================================
  Files         222      222           
  Lines       23387    23393    +6     
  Branches     4073     4074    +1     
=======================================
+ Hits        22309    22315    +6     
  Misses        631      631           
  Partials      447      447           
Files with missing lines Coverage Δ
aaanalysis/data_handling/_load_dataset.py 100.00% <100.00%> (ø)
Components Coverage Δ
cpp_core 95.97% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This branch has not been deployed

No deployments
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