Give each per-variable QRF model its own seed

[vahid-ahmadi]

Fixes #207.

Quantile regression forests (QRF) now use a distinct seed for each target during fitting, target-specific subsampling, and numeric and categorical hyperparameter tuning. This removes the shared random quantile stream that coupled stochastic predictions across targets. QRF(seed=...) controls reproducibility.

Child seeds wrap within scikit-learn's supported range, including when the base seed is 4294967295. The constructor accepts integer seeds from zero through that limit or None, and rejects invalid values, including booleans. An unknown target raises an error when deriving a seeded stream.

Stochastic output changes, so downstream datasets using joint draws need regeneration and checks of joint-tail statistics.

Validation: 114 affected tests passed before the base rebase; verification confirmed that program and test contents remain unchanged. Focused checks covered invalid seeds, boundary derivations, and numeric and categorical tuning. The branch includes the shared lint repair from #218; Ruff 0.16.7 formatting and git diff --check pass.

Checks for current head a8c94bd.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
microimpute-dashboard	Ready	Preview	Sep 21, 2026 10:57am UTC

[vahid-ahmadi]

Reviewed with a fresh pass against the base commit. The bug and the fix both reproduce, and the tests genuinely fail without the change — copying the new test file onto e58c582 gives 14 failed, including the behavioural one: imputed variables are comonotonic; per-variable models are sharing a seed (rank correlations [0.7056, 0.7078, 0.7049]).

The most important finding is about merge order: #219 regresses this fix rather than superseding it. Verified in its diff — its sequential branch ends return self.seed + variable_offset with no modulo and no range validation (grep '2\*\*32' on the diff returns nothing), and it leaves both tuning call sites on self.seed. It also carries only 2 of the 7 tests here. So this should merge first, and #219 rebase onto it.

To do

Before merging

• Make the unknown-variable fallback loud. qrf.py:716-719 catches ValueError and falls back to variable_offset = 0, so any variable not in self.imputed_variables gets byte-identical draws to the first target — this PR's own bug, reintroduced silently. Verified: _seed_for_variable("z") returns 100, same as "a". The call sites all pass post-preprocessing names today, which is exactly why a silent fallback is the wrong shape: it turns a future refactor into a statistical bug with no symptom. Raise, or at minimum logger.warning.
• Validate seed in __init__, not lazily. QRF(seed=-5) and QRF(seed=1.5) both construct fine and only fail mid-fit, where qrf.py:1259-1262 rewraps the ValueError as RuntimeError. That is why the test has to assert RuntimeError. @validate_call is not applied to QRF.__init__, so the Optional[int] annotation buys nothing.

Worth doing

• Correct the PR body's numbers and framing. Measured after the fix is 0.106–0.141, not 0.11 flat. More importantly, the residual is not leftover comonotonicity — all three targets share the conditional mean 0.5*inc, so positive rank correlation is the right answer. "Three targets with nil conditional dependence" is misleading; their conditional dependence is nil but their marginal correlation should not be.
• File a follow-up for MDN. mdn.py:1076, 1106, 1131, 1171 all pass seed=self.seed per variable and mdn.py:601 samples categories from default_rng(self.seed) — the identical mechanism, unfixed. Matching (matching.py:505, 530) is per-variable-seeded too and worth auditing.
• Split out the five docs/** formatter reflows. They are unrelated to seeding and appear byte-identically in Declare subpackages instead of shipping the source tree as package data #216 and Fix pre-submission imputation and evaluation correctness #219, so all three PRs collide on the same files.

Optional

• Derive child seeds with np.random.SeedSequence([seed, offset]).generate_state(1)[0] rather than + offset. Consecutive integers are fine for default_rng, less principled for sklearn's RandomState — and Fix pre-submission imputation and evaluation correctness #219 already uses that idiom for its non-sequential branch.
• isinstance(True, int) is True, so QRF(seed=True) is accepted and yields [1, 2]. Reject bools, or decide it is harmless.
• test_qrf_child_seeds_preserve_existing_seed_values and test_qrf_tuning_uses_distinct_target_seeds assert on a private method, so they test the implementation rather than the behaviour. The behavioural test is the one that would catch a regression through a different mechanism.

The uint32 bound is genuinely required, not defensive: RandomForestClassifier(random_state=2**32) raises InvalidParameterError: must be an int in the range [0, 4294967295].

Note the 8 tests/test_autoimpute.py failures here are pre-existing (NameError: name 'Matching' is not defined, no rpy2) and present on base too — that is #204, fixed in #217.

[juaristi22]

Review of 24b8766: No implementation defect found in this PR. The remaining merge blocker is the inherited Lint failure.

Merge blocker — incorporate #218 and rerun CI. The Lint job fails because Ruff would reformat five unchanged Markdown files: docs/imputation-benchmarking/cross-validation.md, preprocessing.md, visualizations.md, docs/models/imputer/implement-new-model.md, and docs/use_cases/index.md. The workflow runs make check-format. #218 fixes the shared failure by excluding Markdown and aligning the development and CI Ruff ranges at >=0.16.0,<0.17.0. Merge #218 first, update this branch, and require green checks on the resulting head.

Validation at the reviewed head: 114 affected tests passed. Focused seed checks rejected nine invalid inputs, verified four valid derivations, and completed two real Optuna tuning runs at the NumPy uint64 maximum seed. Unit/smoke and changelog CI passed; Lint failed. The current constructor validation, unknown-variable error, bounded child seeds, and per-target tuning streams should all survive the #219 integration.

Integration: read-only merge checks found no textual conflicts in the cumulative #218 + #214 + #215 + #216 + #217 tree. This is not a combined-suite result. #219 conflicts with #214, #215, #217, and #218, so its later rebase must retain the seed, Matching, API, and formatter fixes. The PR description cites an older final commit and historical green checks; update it to the resulting implementation and current validation.

[juaristi22]

Critical fixed

Updated the branch onto current main, incorporating #218's shared lint repair and the 3.1.2 release. Development and CI use Ruff >=0.16.0,<0.17.0, and Markdown documentation is excluded from formatting. The final diff contains only this PR's seed implementation, tests, and changelog fragment.

Should-address fixed

Refreshed the description to match the constructor validation, bounded per-target seeds, unknown-target error, and current validation evidence. Retained the warning that joint stochastic draws change.

Verification

• Source and base-integration verification passed; the rebase preserved program and test contents.
• The earlier affected suite passed 114 tests. The lint repair changed no executable code, so that suite was not redundantly rerun locally.
• Ruff 0.16.7 formatting and git diff --check passed; make format changed no files.

Pushed head: a8c94bd; current-head checks.

GitHub CI: All seven current-head checks passed, including lint, changelog, Python 3.12 smoke tests, Python 3.14 tests with R, and Vercel preview. GitHub reports this pull request as mergeable. (run).