From a7e52d2b0288a268eea5775cae4881bbcd555787 Mon Sep 17 00:00:00 2001 From: Nityahapani Date: Sun, 26 Jul 2026 06:11:52 +0000 Subject: [PATCH] Fix get_params() mutating and being non-idempotent for RNG random_state XGBModel.get_params() converts a np.random.RandomState or np.random.Generator random_state into a plain int for the native learner. It did this by calling .randint() / .integers() directly on the RNG object -- which both (a) mutates the caller's own RNG object as a side effect of what should be a pure accessor call, advancing its internal state, and (b) makes get_params() non-idempotent: calling it twice on the same fitted estimator returns a *different* random_state value each time, since each call draws fresh randomness. This matters because sklearn's own get_params()/clone() contract assumes get_params() is a pure, idempotent accessor -- GridSearchCV, cross_val_score, and any code path that clones or introspects the estimator relies on this. It also silently advances a RandomState/ Generator object that the calling code may still be using elsewhere, which is a surprising side effect for a getter to have. Confirmed via sklearn.base.BaseEstimator.get_params(): the returned dict holds the *same object reference* as self.random_state, not a copy, so .randint()/.integers() genuinely mutates the user's own RNG. Fix: derive a stable seed from the RNG's internal state (read-only) instead of drawing a new random value: - RandomState: use get_state(legacy=True), which returns numpy's classic (str, ndarray, int, int, float) tuple for an MT19937 generator -- verified this is numpy's actual runtime default via help(np.random.RandomState.get_state), even though the .pyi type stub for the no-arg call declares a dict[str, Any] return type (a stub/runtime mismatch); the explicit cast() documents this. - Generator (PCG64): read bit_generator.state['state']['state'] directly, same non-consuming approach. This is the same fix pattern used by at least one other sklearn- compatible library (PySR) for the identical bug: 'fix: potential issue with non-standard random states', switching from a consuming .randint() call to reading get_state()[1][0]. Verified standalone (import xgboost requires the compiled C++ core, which isn't buildable in this sandbox): - RandomState: get_params() logic now returns the identical value on repeated calls (idempotent) - RandomState: the object is provably NOT advanced -- drew a value from a fresh RNG with the same seed before and after running the logic, got identical results - Same two checks repeated for np.random.Generator - Bounds-checked across a range of seeds including edge values (0, 2**31-1, 2**32-1) to confirm results stay in the intended [0, int32_max) range - Plain int random_state values still pass through unchanged mypy and ruff both pass on the modified file. --- python-package/xgboost/sklearn.py | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/python-package/xgboost/sklearn.py b/python-package/xgboost/sklearn.py index afcbec30856c..a4f60407204a 100644 --- a/python-package/xgboost/sklearn.py +++ b/python-package/xgboost/sklearn.py @@ -1076,12 +1076,29 @@ def get_params(self, deep: bool = True) -> Dict[str, Any]: if hasattr(self, "kwargs") and isinstance(self.kwargs, dict): params.update(self.kwargs) if isinstance(params["random_state"], np.random.RandomState): - params["random_state"] = params["random_state"].randint( - np.iinfo(np.int32).max + # Derive a stable seed from the RNG's internal state instead of + # drawing a fresh value with `.randint()`, which would both mutate + # the caller's RandomState object as a side effect and make + # `get_params()` non-idempotent (a different value on every call). + # + # `get_state(legacy=True)` returns the classic + # `(str, ndarray, int, int, float)` tuple for an MT19937-backed + # RandomState (numpy's own default at runtime, verified via + # `help(np.random.RandomState.get_state)`); the `cast` below is + # needed because numpy's type stub for the no-argument overload + # is a `dict[str, Any]`, which doesn't match this actual runtime + # return type. + _, keys, _, _, _ = cast( + Tuple[str, Any, int, int, float], + params["random_state"].get_state(legacy=True), ) + params["random_state"] = int(keys[0]) % np.iinfo(np.int32).max elif isinstance(params["random_state"], np.random.Generator): + # Same rationale as above: read the bit generator's state instead + # of consuming randomness via `.integers()`. params["random_state"] = int( - params["random_state"].integers(np.iinfo(np.int32).max) + params["random_state"].bit_generator.state["state"]["state"] + % np.iinfo(np.int32).max ) return params