diff --git a/python/cuml/cuml/ensemble/isolation_forest.pyx b/python/cuml/cuml/ensemble/isolation_forest.pyx index 4be89eebd0..6489094701 100644 --- a/python/cuml/cuml/ensemble/isolation_forest.pyx +++ b/python/cuml/cuml/ensemble/isolation_forest.pyx @@ -24,7 +24,11 @@ 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 @@ -365,11 +369,6 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): from sklearn.ensemble._iforest import _average_path_length from sklearn.tree import ExtraTreeRegressor - # A failed `fit` can leave `n_features_in_` set (making the model look - # fitted to `InteropMixin`) while no serialized forest exists yet. - if not hasattr(self, "_treelite_model_bytes"): - raise RuntimeError("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) n_features = self.n_features_in_ @@ -642,8 +641,7 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): ------- treelite.Model """ - if not hasattr(self, "_treelite_model_bytes"): - raise RuntimeError("Model has not been fitted. Call fit() first.") + check_is_fitted(self) return treelite.Model.deserialize_bytes(self._treelite_model_bytes) @@ -658,8 +656,7 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): nvforest_model : nvforest.ForestInference A forest inference model that predicts average path length. """ - if not hasattr(self, "_treelite_model_bytes"): - 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), @@ -682,9 +679,6 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): Shared by ``score_samples``, ``decision_function`` and ``predict`` so that input validation runs exactly once per public call. """ - if not hasattr(self, "_treelite_model_bytes"): - raise RuntimeError("Model has not been fitted. Call fit() first.") - nvforest_model = self._get_inference_nvforest_model() dtype = nvforest_model.forest.get_dtype() @@ -746,6 +740,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) + return self._score_samples(X) @mlfunc(preserve_index=True) @@ -766,6 +762,8 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): scores : ndarray of shape (n_samples,) The decision function. Negative values indicate anomalies. """ + check_is_fitted(self) + return self._score_samples(X) - self.offset_ @mlfunc(preserve_index=True) @@ -785,6 +783,8 @@ class IsolationForest(InteropMixin, CMajorInputTagMixin, Base): labels : ndarray of shape (n_samples,) 1 for inliers, -1 for outliers. """ + check_is_fitted(self) + # ``decision_function(X) < 0`` rearranged to avoid materializing it. return cp.where(self._score_samples(X) < self.offset_, -1, 1) diff --git a/python/cuml/tests/test_isolation_forest.py b/python/cuml/tests/test_isolation_forest.py index 8e03a0da67..1b9412b53c 100644 --- a/python/cuml/tests/test_isolation_forest.py +++ b/python/cuml/tests/test_isolation_forest.py @@ -20,6 +20,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 @@ -343,17 +344,6 @@ def test_as_sklearn_respects_max_depth(anomaly_data): ) -def test_as_sklearn_after_failed_fit_raises(blobs_data): - """A failed fit sets ``n_features_in_`` before raising, which makes the - model look fitted to ``InteropMixin``; conversion must still fail - loudly rather than deserialize a missing forest.""" - 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"): - cu_model.as_sklearn() - - @pytest.mark.parametrize( "params", [ @@ -771,10 +761,10 @@ def test_treelite_export_before_fit_raises(): """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() @@ -887,21 +877,16 @@ 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.""" + """score_samples() before fit() should raise an error. + + ``predict`` and ``decision_function`` are covered by sklearn's + ``check_estimators_unfitted``, which never calls ``score_samples``. + """ 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 fitted"): clf.score_samples(X) diff --git a/python/cuml/tests/test_sklearn_compatibility.py b/python/cuml/tests/test_sklearn_compatibility.py index d09d15dd8a..6ea7ac835f 100644 --- a/python/cuml/tests/test_sklearn_compatibility.py +++ b/python/cuml/tests/test_sklearn_compatibility.py @@ -186,11 +186,6 @@ def _all_cuml_estimators(): XFAILS = { - IsolationForest: { - "check_estimators_unfitted": ( - "Unfitted methods raise RuntimeError instead of NotFittedError" - ), - }, KMeans: { "check_sample_weight_equivalence_on_dense_data": "Sample weights not equal to repeating data", },