diff --git a/python/cuml/cuml/ensemble/isolation_forest.pyx b/python/cuml/cuml/ensemble/isolation_forest.pyx index 8270af60d5..309548bba6 100644 --- a/python/cuml/cuml/ensemble/isolation_forest.pyx +++ b/python/cuml/cuml/ensemble/isolation_forest.pyx @@ -18,13 +18,18 @@ 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 InteropMixin, UnsupportedOnGPU 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 @@ -455,6 +460,18 @@ def _isolation_tree_to_sklearn(exported_tree, n_features, n_samples, max_depth): return rebuilt +# 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, +} + + class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): """ GPU-accelerated Isolation Forest for anomaly detection. @@ -661,7 +678,9 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): # A failed `fit` can leave `n_features_in_` set (making the model look # fitted to `InteropMixin`) while no serialized forest exists yet. 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." + ) tl_model = treelite.Model.deserialize_bytes(self._treelite_model_bytes) exported = treelite.sklearn.export_model(tl_model) @@ -699,15 +718,22 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): } 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) + # 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): @@ -935,8 +961,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.") + check_is_fitted(self) return treelite.Model.deserialize_bytes(self._treelite_model_bytes) @@ -951,8 +976,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 RuntimeError("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), @@ -997,8 +1021,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 RuntimeError("Model has not been fitted. Call fit() first.") + check_is_fitted(self) X_m = check_inputs( self, @@ -1050,9 +1073,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 RuntimeError("Model has not been fitted. Call fit() first.") # Convert input to a row-major device array for inference. X_m = check_inputs( @@ -1140,9 +1162,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 RuntimeError("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 fd6ebb08db..de3e3416fd 100644 --- a/python/cuml/tests/test_isolation_forest.py +++ b/python/cuml/tests/test_isolation_forest.py @@ -13,6 +13,7 @@ """ import pickle +import warnings import cupy as cp import numpy as np @@ -20,6 +21,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 UnsupportedOnGPU @@ -341,7 +343,7 @@ def test_as_sklearn_after_failed_fit_raises(blobs_data): cu_model = cuIsolationForest(max_features=0) with pytest.raises(ValueError, match="max_features"): cu_model.fit(blobs_data) - with pytest.raises(RuntimeError, match="not been fitted"): + with pytest.raises(NotFittedError, match="not been fitted"): cu_model.as_sklearn() @@ -780,16 +782,64 @@ 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 fitted"): clf.as_treelite() - with pytest.raises(RuntimeError, match="not been fitted"): + with pytest.raises(NotFittedError, match="not fitted"): clf.as_nvforest() - with pytest.raises(RuntimeError, 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() + + # ============================================================================= # Determinism tests # ============================================================================= @@ -899,24 +949,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(RuntimeError, 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(RuntimeError, 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) diff --git a/python/cuml/tests/test_sklearn_compatibility.py b/python/cuml/tests/test_sklearn_compatibility.py index 6d78dd0059..40f7104ee7 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_estimators_pickle": ( "Pickling does not preserve the fitted model state" ),