From 892a5b6e55dfc886e3eb19932c75446c41b55ca3 Mon Sep 17 00:00:00 2001 From: JulienAu <16043912+JulienAu@users.noreply.github.com> Date: Thu, 13 Aug 2026 10:41:41 +0200 Subject: [PATCH 1/6] Raise NotFittedError from unfitted IsolationForest methods Signed-off-by: JulienAu <16043912+JulienAu@users.noreply.github.com> --- python/cuml/cuml/ensemble/isolation_forest.pyx | 11 ++++++----- python/cuml/tests/test_isolation_forest.py | 11 ++++++----- python/cuml/tests/test_sklearn_compatibility.py | 3 --- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/python/cuml/cuml/ensemble/isolation_forest.pyx b/python/cuml/cuml/ensemble/isolation_forest.pyx index 269317af41..dba051f3ea 100644 --- a/python/cuml/cuml/ensemble/isolation_forest.pyx +++ b/python/cuml/cuml/ensemble/isolation_forest.pyx @@ -18,6 +18,7 @@ import cupy as cp import numpy as np import nvforest import treelite +from sklearn.exceptions import NotFittedError from cuml.internals.base import Base, get_handle from cuml.internals.interop import ( @@ -797,7 +798,7 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): treelite.Model """ if self._treelite_model_bytes is None: - raise RuntimeError("Model has not been fitted. Call fit() first.") + raise NotFittedError("Model has not been fitted. Call fit() first.") return treelite.Model.deserialize_bytes(self._treelite_model_bytes) @@ -813,7 +814,7 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): A forest inference model that predicts average path length. """ if self._treelite_model_bytes is None: - raise RuntimeError("Model has not been fitted. Call fit() first.") + raise NotFittedError("Model has not been fitted. Call fit() first.") return nvforest.load_from_treelite_model( tl_model=treelite.Model.deserialize_bytes(self._treelite_model_bytes), @@ -859,7 +860,7 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): scoring path. """ if self._treelite_model_bytes is None: - raise RuntimeError("Model has not been fitted. Call fit() first.") + raise NotFittedError("Model has not been fitted. Call fit() first.") X_m = check_inputs( self, @@ -913,7 +914,7 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): """ cdef _IsolationForestModel model = self._model if model is None: - raise RuntimeError("Model has not been fitted. Call fit() first.") + raise NotFittedError("Model has not been fitted. Call fit() first.") # Convert input to a row-major device array for inference. X_m = check_inputs( @@ -1003,7 +1004,7 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): """ cdef _IsolationForestModel model = self._model if model is None: - raise RuntimeError("Model has not been fitted. Call fit() first.") + raise NotFittedError("Model has not been fitted. Call fit() first.") # Convert input to a row-major device array for inference. X_m = check_inputs( diff --git a/python/cuml/tests/test_isolation_forest.py b/python/cuml/tests/test_isolation_forest.py index 6e515bb053..dd48f40afa 100644 --- a/python/cuml/tests/test_isolation_forest.py +++ b/python/cuml/tests/test_isolation_forest.py @@ -18,6 +18,7 @@ import treelite from sklearn.datasets import make_blobs from sklearn.ensemble import IsolationForest as skIsolationForest +from sklearn.exceptions import NotFittedError from cuml import IsolationForest as cuIsolationForest from cuml.internals.interop import UnsupportedOnCPU, UnsupportedOnGPU @@ -615,13 +616,13 @@ def test_treelite_export_before_fit_raises(blobs_data): """Treelite and nvForest export should require a fitted model.""" clf = cuIsolationForest() - with pytest.raises(RuntimeError, match="not been fitted"): + with pytest.raises(NotFittedError, match="not been fitted"): clf.as_treelite() - with pytest.raises(RuntimeError, match="not been fitted"): + with pytest.raises(NotFittedError, match="not been fitted"): clf.as_nvforest() - with pytest.raises(RuntimeError, match="not been fitted"): + with pytest.raises(NotFittedError, match="not been fitted"): clf._score_samples_nvforest(blobs_data) @@ -739,7 +740,7 @@ def test_predict_before_fit_raises(): clf = cuIsolationForest() X = np.random.randn(10, 3).astype(np.float32) - with pytest.raises(RuntimeError, match="not been fitted"): + with pytest.raises(NotFittedError, match="not been fitted"): clf.predict(X) @@ -748,7 +749,7 @@ def test_score_samples_before_fit_raises(): clf = cuIsolationForest() X = np.random.randn(10, 3).astype(np.float32) - with pytest.raises(RuntimeError, match="not been fitted"): + with pytest.raises(NotFittedError, match="not been fitted"): clf.score_samples(X) diff --git a/python/cuml/tests/test_sklearn_compatibility.py b/python/cuml/tests/test_sklearn_compatibility.py index de97cfed4d..fb8b5ea7ed 100644 --- a/python/cuml/tests/test_sklearn_compatibility.py +++ b/python/cuml/tests/test_sklearn_compatibility.py @@ -187,9 +187,6 @@ def _all_cuml_estimators(): XFAILS = { IsolationForest: { - "check_estimators_unfitted": ( - "Unfitted methods raise RuntimeError instead of NotFittedError" - ), "check_sample_weights_pandas_series": "Sample weights are not supported", "check_sample_weights_not_an_array": "Sample weights are not supported", "check_sample_weights_list": "Sample weights are not supported", From 9c530969c4249963891dc3777252e7a52de2ea16 Mon Sep 17 00:00:00 2001 From: JulienAu <16043912+JulienAu@users.noreply.github.com> Date: Thu, 13 Aug 2026 14:50:24 +0200 Subject: [PATCH 2/6] Use check_is_fitted for unfitted IsolationForest methods Signed-off-by: JulienAu <16043912+JulienAu@users.noreply.github.com> --- python/cuml/cuml/ensemble/isolation_forest.pyx | 18 +++++++++++++----- python/cuml/tests/test_isolation_forest.py | 18 ------------------ 2 files changed, 13 insertions(+), 23 deletions(-) diff --git a/python/cuml/cuml/ensemble/isolation_forest.pyx b/python/cuml/cuml/ensemble/isolation_forest.pyx index dba051f3ea..5aed163154 100644 --- a/python/cuml/cuml/ensemble/isolation_forest.pyx +++ b/python/cuml/cuml/ensemble/isolation_forest.pyx @@ -29,7 +29,11 @@ from cuml.internals.interop import ( from cuml.internals.mixins import CMajorInputTagMixin from cuml.internals.outputs import mlfunc from cuml.internals.treelite import safe_treelite_call -from cuml.internals.validation import check_inputs, check_random_seed +from cuml.internals.validation import ( + check_inputs, + check_is_fitted, + check_random_seed, +) from libc.stddef cimport size_t from libc.stdint cimport uint64_t, uintptr_t @@ -570,6 +574,12 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): self.__dict__.update(state) @mlfunc(set_input_type=True) + def __sklearn_check_is_fitted__(self): + """Fitted means the native model is present: public attributes + survive unpickling, but the native model does not, and inference + requires it.""" + return self._model is not None + def fit(self, X, y=None, sample_weight=None): """ Fit the Isolation Forest model. @@ -912,9 +922,8 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): Typical range is approximately [-1.0, 0.0], where values below ``offset_`` are predicted as anomalies. """ + check_is_fitted(self) cdef _IsolationForestModel model = self._model - if model is None: - raise NotFittedError("Model has not been fitted. Call fit() first.") # Convert input to a row-major device array for inference. X_m = check_inputs( @@ -1002,9 +1011,8 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): labels : ndarray of shape (n_samples,) 1 for inliers, -1 for outliers. """ + check_is_fitted(self) cdef _IsolationForestModel model = self._model - if model is None: - raise NotFittedError("Model has not been fitted. Call fit() first.") # Convert input to a row-major device array for inference. X_m = check_inputs( diff --git a/python/cuml/tests/test_isolation_forest.py b/python/cuml/tests/test_isolation_forest.py index dd48f40afa..52d5c1b55f 100644 --- a/python/cuml/tests/test_isolation_forest.py +++ b/python/cuml/tests/test_isolation_forest.py @@ -735,24 +735,6 @@ def test_many_features(): assert scores.shape == (X.shape[0],) -def test_predict_before_fit_raises(): - """predict() before fit() should raise an error.""" - clf = cuIsolationForest() - X = np.random.randn(10, 3).astype(np.float32) - - with pytest.raises(NotFittedError, match="not been fitted"): - clf.predict(X) - - -def test_score_samples_before_fit_raises(): - """score_samples() before fit() should raise an error.""" - clf = cuIsolationForest() - X = np.random.randn(10, 3).astype(np.float32) - - with pytest.raises(NotFittedError, match="not been fitted"): - clf.score_samples(X) - - def test_feature_mismatch_raises(blobs_data): """Predicting with wrong number of features should raise.""" clf = cuIsolationForest(n_estimators=10, random_state=42) From 9ca219ab5978451b1b34c7b7bc71322761c6ba8f Mon Sep 17 00:00:00 2001 From: JulienAu <16043912+JulienAu@users.noreply.github.com> Date: Fri, 14 Aug 2026 09:37:19 +0200 Subject: [PATCH 3/6] Fix the fitted hook name and restore the fit decorator Signed-off-by: JulienAu <16043912+JulienAu@users.noreply.github.com> --- python/cuml/cuml/ensemble/isolation_forest.pyx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/cuml/cuml/ensemble/isolation_forest.pyx b/python/cuml/cuml/ensemble/isolation_forest.pyx index 5aed163154..6a52ff77d2 100644 --- a/python/cuml/cuml/ensemble/isolation_forest.pyx +++ b/python/cuml/cuml/ensemble/isolation_forest.pyx @@ -573,13 +573,13 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): """Pickle support - restore state.""" self.__dict__.update(state) - @mlfunc(set_input_type=True) - def __sklearn_check_is_fitted__(self): + def __sklearn_is_fitted__(self): """Fitted means the native model is present: public attributes survive unpickling, but the native model does not, and inference requires it.""" return self._model is not None + @mlfunc(set_input_type=True) def fit(self, X, y=None, sample_weight=None): """ Fit the Isolation Forest model. From b3e7e9c21a68108c02f0805590639d35d69eff19 Mon Sep 17 00:00:00 2001 From: JulienAu <16043912+JulienAu@users.noreply.github.com> Date: Tue, 18 Aug 2026 15:54:15 +0200 Subject: [PATCH 4/6] Make an unpickled IsolationForest genuinely unfitted The native model cannot be serialized, so pickling previously produced a half fitted estimator: public attributes and the treelite bytes survived while inference was broken. __getstate__ now drops the fitted state entirely, keeping only constructor parameters, and warns when fitted state is lost. This removes the need for the __sklearn_is_fitted__ hook: plain check_is_fitted now gates every method, including the treelite and nvforest exports. fit() also resets the fitted state up front and on failure, so an estimator whose fit raised is genuinely unfitted instead of exposing n_features_in_ and other attributes from the aborted call. Signed-off-by: JulienAu <16043912+JulienAu@users.noreply.github.com> --- .../cuml/cuml/ensemble/isolation_forest.pyx | 326 ++++++++++-------- python/cuml/tests/test_isolation_forest.py | 71 +++- 2 files changed, 243 insertions(+), 154 deletions(-) diff --git a/python/cuml/cuml/ensemble/isolation_forest.pyx b/python/cuml/cuml/ensemble/isolation_forest.pyx index 6a52ff77d2..b6b51f3979 100644 --- a/python/cuml/cuml/ensemble/isolation_forest.pyx +++ b/python/cuml/cuml/ensemble/isolation_forest.pyx @@ -18,7 +18,6 @@ import cupy as cp import numpy as np import nvforest import treelite -from sklearn.exceptions import NotFittedError from cuml.internals.base import Base, get_handle from cuml.internals.interop import ( @@ -357,6 +356,27 @@ cdef class _IsolationForestModelFloat64(_IsolationForestModel): ) +# Baseline values (matching ``__init__``) restored whenever the fitted state +# is dropped. +_UNFITTED_BASELINE = { + "_model": None, + "_dtype": None, + "_treelite_model_bytes": None, + "_nvforest_model": None, + "_c_normalization": None, + "_n_features_per_tree": None, +} + +# Attributes that only exist on a fitted estimator. +_FITTED_ONLY_ATTRS = ( + "n_features_in_", + "feature_names_in_", + "max_samples_", + "offset_", + "_n_samples_per_tree", +) + + class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): """ GPU-accelerated Isolation Forest for anomaly detection. @@ -557,28 +577,32 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): "Conversion of a fitted cuML IsolationForest is not supported" ) + def _reset_fitted_state(self): + """Returns the estimator to its unfitted construction state.""" + self.__dict__.update(_UNFITTED_BASELINE) + for attr in _FITTED_ONLY_ATTRS: + self.__dict__.pop(attr, None) + def __getstate__(self): - """Pickle support - serialize state.""" + """Pickle support: the native model cannot be serialized, so the + fitted state is dropped entirely and the unpickled estimator is + unfitted, keeping only its constructor parameters.""" state = self.__dict__.copy() - # The native model is not currently serialized. - state["_model"] = None - state.pop("_nvforest_model", None) - warnings.warn( - "IsolationForest model serialization is not fully supported. " - "The model will need to be re-fitted after unpickling." - ) + if self._model is not None: + warnings.warn( + "cuML IsolationForest does not serialize its fitted " + "state. The unpickled estimator is unfitted; call fit() " + "again before using it." + ) + state.update(_UNFITTED_BASELINE) + for attr in _FITTED_ONLY_ATTRS: + state.pop(attr, None) return state def __setstate__(self, state): """Pickle support - restore state.""" self.__dict__.update(state) - def __sklearn_is_fitted__(self): - """Fitted means the native model is present: public attributes - survive unpickling, but the native model does not, and inference - requires it.""" - return self._model is not None - @mlfunc(set_input_type=True) def fit(self, X, y=None, sample_weight=None): """ @@ -604,145 +628,151 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): if sample_weight is not None: raise UnsupportedOnGPU("`sample_weight` is not supported") - # Release any existing native model. - self._model = None - - # Convert input to a column-major device array for fit. - X_m = check_inputs( - self, - X, - dtype=(np.float32, np.float64), - order="F", - reset=True, - ) + # Drop any previous fitted state up front, so that a failed fit + # leaves the estimator unfitted rather than half fitted. + self._reset_fitted_state() - cdef size_t n_rows = X_m.shape[0] - cdef int n_cols = X_m.shape[1] - cdef uintptr_t X_ptr = X_m.data.ptr + cdef size_t n_rows + cdef int n_cols + cdef uintptr_t X_ptr cdef double contamination_fraction = 0.0 cdef bint use_contamination_quantile = False - self.n_features_in_ = n_cols - self._dtype = X_m.dtype - cdef int actual_max_features - if isinstance(self.max_features, builtins.bool): - raise ValueError( - "max_features must be an int in [1, n_features] or a float " - "in (0.0, 1.0]." + cdef int actual_max_samples + cdef int actual_max_depth + cdef uint64_t seed + cdef IF_params params + cdef handle_t* handle_ + cdef level_enum verbose + cdef _IsolationForestModel model + cdef TreeliteModelHandle tl_handle = NULL + cdef const char* tl_bytes = NULL + cdef size_t tl_bytes_len + cdef int tl_free_status + + try: + # Convert input to a column-major device array for fit. + # ``reset=True`` sets ``n_features_in_``/``feature_names_in_``. + X_m = check_inputs( + self, + X, + dtype=(np.float32, np.float64), + order="F", + reset=True, ) - elif isinstance(self.max_features, Integral): - if self.max_features < 1 or self.max_features > n_cols: - raise ValueError( - "max_features must be an int in [1, n_features] or a " - "float in (0.0, 1.0]." - ) - actual_max_features = int(self.max_features) - elif isinstance(self.max_features, Real): - if self.max_features <= 0.0 or self.max_features > 1.0: + n_rows = X_m.shape[0] + n_cols = X_m.shape[1] + X_ptr = X_m.data.ptr + self._dtype = X_m.dtype + + if isinstance(self.max_features, builtins.bool): raise ValueError( - "max_features must be an int in [1, n_features] or a " - "float in (0.0, 1.0]." + "max_features must be an int in [1, n_features] or a float " + "in (0.0, 1.0]." ) - actual_max_features = max(1, int(self.max_features * n_cols)) - else: - raise ValueError( - "max_features must be an int in [1, n_features] or a float " - "in (0.0, 1.0]." - ) - self._n_features_per_tree = actual_max_features - - if isinstance(self.contamination, str): - if self.contamination != "auto": + elif isinstance(self.max_features, Integral): + if self.max_features < 1 or self.max_features > n_cols: + raise ValueError( + "max_features must be an int in [1, n_features] or a " + "float in (0.0, 1.0]." + ) + actual_max_features = int(self.max_features) + elif isinstance(self.max_features, Real): + if self.max_features <= 0.0 or self.max_features > 1.0: + raise ValueError( + "max_features must be an int in [1, n_features] or a " + "float in (0.0, 1.0]." + ) + actual_max_features = max(1, int(self.max_features * n_cols)) + else: raise ValueError( - "contamination must be 'auto' or a float in the range " - "(0, 0.5]." + "max_features must be an int in [1, n_features] or a float " + "in (0.0, 1.0]." ) - elif isinstance(self.contamination, Real): - contamination_fraction = float(self.contamination) - if contamination_fraction <= 0.0 or contamination_fraction > 0.5: + self._n_features_per_tree = actual_max_features + + if isinstance(self.contamination, str): + if self.contamination != "auto": + raise ValueError( + "contamination must be 'auto' or a float in the range " + "(0, 0.5]." + ) + elif isinstance(self.contamination, Real): + contamination_fraction = float(self.contamination) + if contamination_fraction <= 0.0 or contamination_fraction > 0.5: + raise ValueError( + "contamination must be 'auto' or a float in the range " + "(0, 0.5]." + ) + use_contamination_quantile = True + else: raise ValueError( "contamination must be 'auto' or a float in the range " "(0, 0.5]." ) - use_contamination_quantile = True - else: - raise ValueError( - "contamination must be 'auto' or a float in the range " - "(0, 0.5]." - ) - # Compute max_samples - cdef int actual_max_samples - if isinstance(self.max_samples, str): - if self.max_samples != "auto": + # Compute max_samples + if isinstance(self.max_samples, str): + if self.max_samples != "auto": + raise ValueError( + "max_samples must be 'auto', a positive int, or a float " + "in (0.0, 1.0]." + ) + actual_max_samples = min(256, n_rows) + elif isinstance(self.max_samples, builtins.bool): raise ValueError( "max_samples must be 'auto', a positive int, or a float " "in (0.0, 1.0]." ) - actual_max_samples = min(256, n_rows) - elif isinstance(self.max_samples, builtins.bool): - raise ValueError( - "max_samples must be 'auto', a positive int, or a float " - "in (0.0, 1.0]." - ) - elif isinstance(self.max_samples, Integral): - if self.max_samples <= 0: - raise ValueError("max_samples must be a positive integer.") - if self.max_samples > n_rows: - warnings.warn( - f"max_samples ({self.max_samples}) is greater than the " - f"total number of samples ({n_rows}). max_samples will " - "be set to n_samples for estimation.", - UserWarning, - ) - actual_max_samples = min(self.max_samples, n_rows) - elif isinstance(self.max_samples, Real): - if self.max_samples <= 0.0 or self.max_samples > 1.0: - raise ValueError("float max_samples must be in (0.0, 1.0].") - actual_max_samples = int(self.max_samples * n_rows) - if actual_max_samples < 1: + elif isinstance(self.max_samples, Integral): + if self.max_samples <= 0: + raise ValueError("max_samples must be a positive integer.") + if self.max_samples > n_rows: + warnings.warn( + f"max_samples ({self.max_samples}) is greater than the " + f"total number of samples ({n_rows}). max_samples will " + "be set to n_samples for estimation.", + UserWarning, + ) + actual_max_samples = min(self.max_samples, n_rows) + elif isinstance(self.max_samples, Real): + if self.max_samples <= 0.0 or self.max_samples > 1.0: + raise ValueError("float max_samples must be in (0.0, 1.0].") + actual_max_samples = int(self.max_samples * n_rows) + if actual_max_samples < 1: + raise ValueError( + "max_samples resolves to 0 samples; increase max_samples " + "or provide more training rows." + ) + else: raise ValueError( - "max_samples resolves to 0 samples; increase max_samples " - "or provide more training rows." + "max_samples must be 'auto', a positive int, or a float " + "in (0.0, 1.0]." ) - else: - raise ValueError( - "max_samples must be 'auto', a positive int, or a float " - "in (0.0, 1.0]." - ) - self.max_samples_ = actual_max_samples + self.max_samples_ = actual_max_samples - # Compute max_depth (-1 means auto in C++) - cdef int actual_max_depth - if self.max_depth is None: - actual_max_depth = -1 # C++ will compute ceil(log2(max_samples)) - else: - actual_max_depth = self.max_depth - - # Get random seed - cdef uint64_t seed = check_random_seed(self.random_state) + # Compute max_depth (-1 means auto in C++) + if self.max_depth is None: + actual_max_depth = -1 # C++ will compute ceil(log2(max_samples)) + else: + actual_max_depth = self.max_depth - # Setup parameters - cdef IF_params params - params.n_estimators = self.n_estimators - params.max_samples = actual_max_samples - params.max_depth = actual_max_depth - params.max_features = actual_max_features - params.bootstrap = self.bootstrap - params.seed = seed + # Get random seed + seed = check_random_seed(self.random_state) - # Get handle and verbosity - handle = get_handle() - cdef handle_t* handle_ = handle.getHandle() - cdef level_enum verbose = self._verbose_level + # Setup parameters + params.n_estimators = self.n_estimators + params.max_samples = actual_max_samples + params.max_depth = actual_max_depth + params.max_features = actual_max_features + params.bootstrap = self.bootstrap + params.seed = seed - cdef _IsolationForestModel model - cdef TreeliteModelHandle tl_handle = NULL - cdef const char* tl_bytes = NULL - cdef size_t tl_bytes_len - cdef int tl_free_status + # Get handle and verbosity + handle = get_handle() + handle_ = handle.getHandle() + verbose = self._verbose_level - try: if X_m.dtype == np.float32: model = _IsolationForestModelFloat32() else: @@ -773,27 +803,24 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): safe_treelite_call( tl_free_status, "Failed to free Treelite model:" ) + self._treelite_model_bytes = (tl_bytes[:tl_bytes_len]) + self._nvforest_model = None + + if use_contamination_quantile: + training_scores = self.score_samples(X_m) + self.offset_ = float( + cp.percentile( + training_scores, 100.0 * contamination_fraction + ).get() + ) + else: + self.offset_ = -0.5 except Exception: if tl_handle != NULL: TreeliteFreeModel(tl_handle) - self._model = None - self._treelite_model_bytes = None - self._nvforest_model = None + self._reset_fitted_state() raise - self._treelite_model_bytes = (tl_bytes[:tl_bytes_len]) - self._nvforest_model = None - - if use_contamination_quantile: - training_scores = self.score_samples(X_m) - self.offset_ = float( - cp.percentile( - training_scores, 100.0 * contamination_fraction - ).get() - ) - else: - self.offset_ = -0.5 - return self def as_treelite(self): @@ -807,8 +834,7 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): ------- treelite.Model """ - if self._treelite_model_bytes is None: - raise NotFittedError("Model has not been fitted. Call fit() first.") + check_is_fitted(self) return treelite.Model.deserialize_bytes(self._treelite_model_bytes) @@ -823,8 +849,7 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): nvforest_model : nvforest.ForestInference A forest inference model that predicts average path length. """ - if self._treelite_model_bytes is None: - raise NotFittedError("Model has not been fitted. Call fit() first.") + check_is_fitted(self) return nvforest.load_from_treelite_model( tl_model=treelite.Model.deserialize_bytes(self._treelite_model_bytes), @@ -869,8 +894,7 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): are added. Public ``score_samples`` continues to use the existing C++ scoring path. """ - if self._treelite_model_bytes is None: - raise NotFittedError("Model has not been fitted. Call fit() first.") + check_is_fitted(self) X_m = check_inputs( self, diff --git a/python/cuml/tests/test_isolation_forest.py b/python/cuml/tests/test_isolation_forest.py index 52d5c1b55f..9d0098ab5f 100644 --- a/python/cuml/tests/test_isolation_forest.py +++ b/python/cuml/tests/test_isolation_forest.py @@ -12,6 +12,9 @@ 4. Validating sklearn compatibility """ +import pickle +import warnings + import cupy as cp import numpy as np import pytest @@ -616,16 +619,78 @@ def test_treelite_export_before_fit_raises(blobs_data): """Treelite and nvForest export should require a fitted model.""" clf = cuIsolationForest() - with pytest.raises(NotFittedError, match="not been fitted"): + with pytest.raises(NotFittedError, match="not fitted"): clf.as_treelite() - with pytest.raises(NotFittedError, match="not been fitted"): + with pytest.raises(NotFittedError, match="not fitted"): clf.as_nvforest() - with pytest.raises(NotFittedError, match="not been fitted"): + with pytest.raises(NotFittedError, match="not fitted"): clf._score_samples_nvforest(blobs_data) +# ============================================================================= +# Pickling tests +# ============================================================================= + + +def _fitted_only_attrs(estimator): + return sorted( + attr + for attr in vars(estimator) + if attr.endswith("_") and not attr.startswith("__") + ) + + +def test_pickle_fitted_model_is_unfitted_after_roundtrip(blobs_data): + """The native model cannot be serialized: pickling a fitted model warns + and unpickles as a genuinely unfitted estimator with the same + parameters.""" + clf = cuIsolationForest(n_estimators=10, random_state=42).fit(blobs_data) + + with pytest.warns(UserWarning, match="unfitted"): + payload = pickle.dumps(clf) + loaded = pickle.loads(payload) + + assert _fitted_only_attrs(loaded) == [] + assert loaded._treelite_model_bytes is None + assert loaded.get_params() == clf.get_params() + with pytest.raises(NotFittedError, match="not fitted"): + loaded.predict(blobs_data) + with pytest.raises(NotFittedError, match="not fitted"): + loaded.as_treelite() + + # Refitting the unpickled estimator reproduces the original model. + refit_scores = np.asarray(loaded.fit(blobs_data).score_samples(blobs_data)) + original_scores = np.asarray(clf.score_samples(blobs_data)) + np.testing.assert_allclose(refit_scores, original_scores) + + +def test_pickle_unfitted_model_is_silent(): + """Pickling an unfitted estimator round trips without warning.""" + clf = cuIsolationForest(n_estimators=7, random_state=3) + + with warnings.catch_warnings(): + warnings.simplefilter("error") + loaded = pickle.loads(pickle.dumps(clf)) + + assert loaded.get_params() == clf.get_params() + + +def test_failed_fit_leaves_estimator_unfitted(blobs_data): + """A failed fit must leave the estimator genuinely unfitted rather than + half fitted.""" + clf = cuIsolationForest(n_estimators=10, random_state=42).fit(blobs_data) + clf.max_features = 0 + + with pytest.raises(ValueError, match="max_features"): + clf.fit(blobs_data) + + assert _fitted_only_attrs(clf) == [] + with pytest.raises(NotFittedError, match="not fitted"): + clf.predict(blobs_data) + + # ============================================================================= # Determinism tests # ============================================================================= From 91ef491187cfbcb6d8d48ed57b8a76ba618239a8 Mon Sep 17 00:00:00 2001 From: JulienAu <16043912+JulienAu@users.noreply.github.com> Date: Tue, 18 Aug 2026 16:01:45 +0200 Subject: [PATCH 5/6] Share one implementation for clearing fitted state Signed-off-by: JulienAu <16043912+JulienAu@users.noreply.github.com> --- python/cuml/cuml/ensemble/isolation_forest.pyx | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/python/cuml/cuml/ensemble/isolation_forest.pyx b/python/cuml/cuml/ensemble/isolation_forest.pyx index b6b51f3979..d0a1113bbf 100644 --- a/python/cuml/cuml/ensemble/isolation_forest.pyx +++ b/python/cuml/cuml/ensemble/isolation_forest.pyx @@ -577,11 +577,17 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): "Conversion of a fitted cuML IsolationForest is not supported" ) + @staticmethod + def _clear_fitted_state(state): + """Applies the unfitted baseline to the ``state`` mapping in place.""" + state.update(_UNFITTED_BASELINE) + for attr in _FITTED_ONLY_ATTRS: + state.pop(attr, None) + return state + def _reset_fitted_state(self): """Returns the estimator to its unfitted construction state.""" - self.__dict__.update(_UNFITTED_BASELINE) - for attr in _FITTED_ONLY_ATTRS: - self.__dict__.pop(attr, None) + self._clear_fitted_state(self.__dict__) def __getstate__(self): """Pickle support: the native model cannot be serialized, so the @@ -594,10 +600,7 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): "state. The unpickled estimator is unfitted; call fit() " "again before using it." ) - state.update(_UNFITTED_BASELINE) - for attr in _FITTED_ONLY_ATTRS: - state.pop(attr, None) - return state + return self._clear_fitted_state(state) def __setstate__(self, state): """Pickle support - restore state.""" From 946e3cea135f93da3ea8f169c345964873580266 Mon Sep 17 00:00:00 2001 From: JulienAu <16043912+JulienAu@users.noreply.github.com> Date: Tue, 18 Aug 2026 18:20:23 +0200 Subject: [PATCH 6/6] Simplify per review: original fit flow, dynamic fitted-attribute pickling fit is restored to its reviewed structure: fitted attributes are simply overwritten by a second fit and the state after a raising fit is not specified. __getstate__ now drops fitted attributes by the sklearn naming convention (trailing underscore, non leading underscore) instead of a hardcoded list, on top of resetting the private native-model fields. Signed-off-by: JulienAu <16043912+JulienAu@users.noreply.github.com> --- .../cuml/cuml/ensemble/isolation_forest.pyx | 292 ++++++++---------- python/cuml/tests/test_isolation_forest.py | 14 - 2 files changed, 137 insertions(+), 169 deletions(-) diff --git a/python/cuml/cuml/ensemble/isolation_forest.pyx b/python/cuml/cuml/ensemble/isolation_forest.pyx index d0a1113bbf..53e916854b 100644 --- a/python/cuml/cuml/ensemble/isolation_forest.pyx +++ b/python/cuml/cuml/ensemble/isolation_forest.pyx @@ -367,15 +367,6 @@ _UNFITTED_BASELINE = { "_n_features_per_tree": None, } -# Attributes that only exist on a fitted estimator. -_FITTED_ONLY_ATTRS = ( - "n_features_in_", - "feature_names_in_", - "max_samples_", - "offset_", - "_n_samples_per_tree", -) - class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): """ @@ -577,18 +568,6 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): "Conversion of a fitted cuML IsolationForest is not supported" ) - @staticmethod - def _clear_fitted_state(state): - """Applies the unfitted baseline to the ``state`` mapping in place.""" - state.update(_UNFITTED_BASELINE) - for attr in _FITTED_ONLY_ATTRS: - state.pop(attr, None) - return state - - def _reset_fitted_state(self): - """Returns the estimator to its unfitted construction state.""" - self._clear_fitted_state(self.__dict__) - def __getstate__(self): """Pickle support: the native model cannot be serialized, so the fitted state is dropped entirely and the unpickled estimator is @@ -600,7 +579,13 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): "state. The unpickled estimator is unfitted; call fit() " "again before using it." ) - return self._clear_fitted_state(state) + state.update(_UNFITTED_BASELINE) + # Fitted attributes follow the sklearn naming convention. + for attr in list(state): + if attr.endswith("_") and not attr.startswith("_"): + del state[attr] + state.pop("_n_samples_per_tree", None) + return state def __setstate__(self, state): """Pickle support - restore state.""" @@ -631,151 +616,145 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): if sample_weight is not None: raise UnsupportedOnGPU("`sample_weight` is not supported") - # Drop any previous fitted state up front, so that a failed fit - # leaves the estimator unfitted rather than half fitted. - self._reset_fitted_state() + # Release any existing native model. + self._model = None - cdef size_t n_rows - cdef int n_cols - cdef uintptr_t X_ptr + # Convert input to a column-major device array for fit. + X_m = check_inputs( + self, + X, + dtype=(np.float32, np.float64), + order="F", + reset=True, + ) + + cdef size_t n_rows = X_m.shape[0] + cdef int n_cols = X_m.shape[1] + cdef uintptr_t X_ptr = X_m.data.ptr cdef double contamination_fraction = 0.0 cdef bint use_contamination_quantile = False - cdef int actual_max_features - cdef int actual_max_samples - cdef int actual_max_depth - cdef uint64_t seed - cdef IF_params params - cdef handle_t* handle_ - cdef level_enum verbose - cdef _IsolationForestModel model - cdef TreeliteModelHandle tl_handle = NULL - cdef const char* tl_bytes = NULL - cdef size_t tl_bytes_len - cdef int tl_free_status + self.n_features_in_ = n_cols + self._dtype = X_m.dtype - try: - # Convert input to a column-major device array for fit. - # ``reset=True`` sets ``n_features_in_``/``feature_names_in_``. - X_m = check_inputs( - self, - X, - dtype=(np.float32, np.float64), - order="F", - reset=True, + cdef int actual_max_features + if isinstance(self.max_features, builtins.bool): + raise ValueError( + "max_features must be an int in [1, n_features] or a float " + "in (0.0, 1.0]." ) - n_rows = X_m.shape[0] - n_cols = X_m.shape[1] - X_ptr = X_m.data.ptr - self._dtype = X_m.dtype - - if isinstance(self.max_features, builtins.bool): + elif isinstance(self.max_features, Integral): + if self.max_features < 1 or self.max_features > n_cols: raise ValueError( - "max_features must be an int in [1, n_features] or a float " - "in (0.0, 1.0]." + "max_features must be an int in [1, n_features] or a " + "float in (0.0, 1.0]." ) - elif isinstance(self.max_features, Integral): - if self.max_features < 1 or self.max_features > n_cols: - raise ValueError( - "max_features must be an int in [1, n_features] or a " - "float in (0.0, 1.0]." - ) - actual_max_features = int(self.max_features) - elif isinstance(self.max_features, Real): - if self.max_features <= 0.0 or self.max_features > 1.0: - raise ValueError( - "max_features must be an int in [1, n_features] or a " - "float in (0.0, 1.0]." - ) - actual_max_features = max(1, int(self.max_features * n_cols)) - else: + actual_max_features = int(self.max_features) + elif isinstance(self.max_features, Real): + if self.max_features <= 0.0 or self.max_features > 1.0: raise ValueError( - "max_features must be an int in [1, n_features] or a float " - "in (0.0, 1.0]." + "max_features must be an int in [1, n_features] or a " + "float in (0.0, 1.0]." ) - self._n_features_per_tree = actual_max_features - - if isinstance(self.contamination, str): - if self.contamination != "auto": - raise ValueError( - "contamination must be 'auto' or a float in the range " - "(0, 0.5]." - ) - elif isinstance(self.contamination, Real): - contamination_fraction = float(self.contamination) - if contamination_fraction <= 0.0 or contamination_fraction > 0.5: - raise ValueError( - "contamination must be 'auto' or a float in the range " - "(0, 0.5]." - ) - use_contamination_quantile = True - else: + actual_max_features = max(1, int(self.max_features * n_cols)) + else: + raise ValueError( + "max_features must be an int in [1, n_features] or a float " + "in (0.0, 1.0]." + ) + self._n_features_per_tree = actual_max_features + + if isinstance(self.contamination, str): + if self.contamination != "auto": raise ValueError( "contamination must be 'auto' or a float in the range " "(0, 0.5]." ) + elif isinstance(self.contamination, Real): + contamination_fraction = float(self.contamination) + if contamination_fraction <= 0.0 or contamination_fraction > 0.5: + raise ValueError( + "contamination must be 'auto' or a float in the range " + "(0, 0.5]." + ) + use_contamination_quantile = True + else: + raise ValueError( + "contamination must be 'auto' or a float in the range " + "(0, 0.5]." + ) - # Compute max_samples - if isinstance(self.max_samples, str): - if self.max_samples != "auto": - raise ValueError( - "max_samples must be 'auto', a positive int, or a float " - "in (0.0, 1.0]." - ) - actual_max_samples = min(256, n_rows) - elif isinstance(self.max_samples, builtins.bool): + # Compute max_samples + cdef int actual_max_samples + if isinstance(self.max_samples, str): + if self.max_samples != "auto": raise ValueError( "max_samples must be 'auto', a positive int, or a float " "in (0.0, 1.0]." ) - elif isinstance(self.max_samples, Integral): - if self.max_samples <= 0: - raise ValueError("max_samples must be a positive integer.") - if self.max_samples > n_rows: - warnings.warn( - f"max_samples ({self.max_samples}) is greater than the " - f"total number of samples ({n_rows}). max_samples will " - "be set to n_samples for estimation.", - UserWarning, - ) - actual_max_samples = min(self.max_samples, n_rows) - elif isinstance(self.max_samples, Real): - if self.max_samples <= 0.0 or self.max_samples > 1.0: - raise ValueError("float max_samples must be in (0.0, 1.0].") - actual_max_samples = int(self.max_samples * n_rows) - if actual_max_samples < 1: - raise ValueError( - "max_samples resolves to 0 samples; increase max_samples " - "or provide more training rows." - ) - else: + actual_max_samples = min(256, n_rows) + elif isinstance(self.max_samples, builtins.bool): + raise ValueError( + "max_samples must be 'auto', a positive int, or a float " + "in (0.0, 1.0]." + ) + elif isinstance(self.max_samples, Integral): + if self.max_samples <= 0: + raise ValueError("max_samples must be a positive integer.") + if self.max_samples > n_rows: + warnings.warn( + f"max_samples ({self.max_samples}) is greater than the " + f"total number of samples ({n_rows}). max_samples will " + "be set to n_samples for estimation.", + UserWarning, + ) + actual_max_samples = min(self.max_samples, n_rows) + elif isinstance(self.max_samples, Real): + if self.max_samples <= 0.0 or self.max_samples > 1.0: + raise ValueError("float max_samples must be in (0.0, 1.0].") + actual_max_samples = int(self.max_samples * n_rows) + if actual_max_samples < 1: raise ValueError( - "max_samples must be 'auto', a positive int, or a float " - "in (0.0, 1.0]." + "max_samples resolves to 0 samples; increase max_samples " + "or provide more training rows." ) - self.max_samples_ = actual_max_samples + else: + raise ValueError( + "max_samples must be 'auto', a positive int, or a float " + "in (0.0, 1.0]." + ) + self.max_samples_ = actual_max_samples - # Compute max_depth (-1 means auto in C++) - if self.max_depth is None: - actual_max_depth = -1 # C++ will compute ceil(log2(max_samples)) - else: - actual_max_depth = self.max_depth + # Compute max_depth (-1 means auto in C++) + cdef int actual_max_depth + if self.max_depth is None: + actual_max_depth = -1 # C++ will compute ceil(log2(max_samples)) + else: + actual_max_depth = self.max_depth - # Get random seed - seed = check_random_seed(self.random_state) + # Get random seed + cdef uint64_t seed = check_random_seed(self.random_state) - # Setup parameters - params.n_estimators = self.n_estimators - params.max_samples = actual_max_samples - params.max_depth = actual_max_depth - params.max_features = actual_max_features - params.bootstrap = self.bootstrap - params.seed = seed + # Setup parameters + cdef IF_params params + params.n_estimators = self.n_estimators + params.max_samples = actual_max_samples + params.max_depth = actual_max_depth + params.max_features = actual_max_features + params.bootstrap = self.bootstrap + params.seed = seed - # Get handle and verbosity - handle = get_handle() - handle_ = handle.getHandle() - verbose = self._verbose_level + # Get handle and verbosity + handle = get_handle() + cdef handle_t* handle_ = handle.getHandle() + cdef level_enum verbose = self._verbose_level + cdef _IsolationForestModel model + cdef TreeliteModelHandle tl_handle = NULL + cdef const char* tl_bytes = NULL + cdef size_t tl_bytes_len + cdef int tl_free_status + + try: if X_m.dtype == np.float32: model = _IsolationForestModelFloat32() else: @@ -806,24 +785,27 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): safe_treelite_call( tl_free_status, "Failed to free Treelite model:" ) - self._treelite_model_bytes = (tl_bytes[:tl_bytes_len]) - self._nvforest_model = None - - if use_contamination_quantile: - training_scores = self.score_samples(X_m) - self.offset_ = float( - cp.percentile( - training_scores, 100.0 * contamination_fraction - ).get() - ) - else: - self.offset_ = -0.5 except Exception: if tl_handle != NULL: TreeliteFreeModel(tl_handle) - self._reset_fitted_state() + self._model = None + self._treelite_model_bytes = None + self._nvforest_model = None raise + self._treelite_model_bytes = (tl_bytes[:tl_bytes_len]) + self._nvforest_model = None + + if use_contamination_quantile: + training_scores = self.score_samples(X_m) + self.offset_ = float( + cp.percentile( + training_scores, 100.0 * contamination_fraction + ).get() + ) + else: + self.offset_ = -0.5 + return self def as_treelite(self): diff --git a/python/cuml/tests/test_isolation_forest.py b/python/cuml/tests/test_isolation_forest.py index 9d0098ab5f..adbbc9ea67 100644 --- a/python/cuml/tests/test_isolation_forest.py +++ b/python/cuml/tests/test_isolation_forest.py @@ -677,20 +677,6 @@ def test_pickle_unfitted_model_is_silent(): assert loaded.get_params() == clf.get_params() -def test_failed_fit_leaves_estimator_unfitted(blobs_data): - """A failed fit must leave the estimator genuinely unfitted rather than - half fitted.""" - clf = cuIsolationForest(n_estimators=10, random_state=42).fit(blobs_data) - clf.max_features = 0 - - with pytest.raises(ValueError, match="max_features"): - clf.fit(blobs_data) - - assert _fitted_only_attrs(clf) == [] - with pytest.raises(NotFittedError, match="not fitted"): - clf.predict(blobs_data) - - # ============================================================================= # Determinism tests # =============================================================================