Normalize meta-learner random_state handling through check_random_state
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 6k
- Forks
- 877
- PR merge metrics
- No merged PRs in 30d
Description
Prerequisite for #1036 — that issue threads this normalization through the S- and X-learner families, so this one should land first.
#1032 introduced check_random_state to rlearner.py (:7, used at :316 and :421). It is now the only meta-learner that does, which leaves two concrete inconsistencies on master.
a. BaseLearner.bootstrap's rng parameter is now documented wrongly. base.py:128 says rng (np.random.Generator, optional), but as of #1032 rlearner.py:316 passes it the RandomState returned by check_random_state. Both work — .choice() exists on each — but the docstring no longer describes what the only caller supplies.
b. base.py still uses the constructor rather than the normalizer. base.py:40 (_fit_bootstrap_clone) and base.py:186 (fit_bootstrap_ensemble) call np.random.RandomState(random_state), which rejects a RandomState instance:
np.random.RandomState(np.random.RandomState(42))
# TypeError: Cannot cast scalar from dtype('O') to dtype('int64') according to the rule 'safe'
base.py:180 documents that parameter as int, optional, so it is self-consistent — but base.py:292 and :365 document random_state (int/RandomState instance/None) for permutation importance, so base.py is not internally consistent either.
The tree side already follows scikit-learn's convention: match.py:143, tree/_tree/_classes.py:233, causalforest.py:482, causal/_tree.py:101, upliftforest.py:66.
scikit-learn's Controlling randomness guidance is to pass RandomState instances to estimators and ints to CV splitters, and scikit-learn does not support numpy Generator at all — check_random_state(np.random.default_rng(42)) raises ValueError on scikit-learn 1.7.0.
Proposed:
- Route every meta-learner RNG through
check_random_state. - Widen
base.py:128tonp.random.Generator or np.random.RandomState. - Widen
base.py:180toint or RandomStateonce it holds.
No API narrowing — this widens accepted input.
Observed on master (477cd0a).
🤖 Generated with Claude Code
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with base.py at _fit_bootstrap_clone, fit_bootstrap_ensemble, and the documented random_state parameters, then compare the existing handling in rlearner.py and the tree-side files listed in the issue. Trace every meta-learner RNG entry point and run the relevant test suite. Done means all meta-learners use check_random_state and the base.py documentation matches the accepted inputs.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- numpy, python, scikit-learn
- Domain
- machine-learning
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100