[MRG] iforest chunks for score_samples#13283
Conversation
|
@ngoix please put the PR title to MRG when ready on your side |
tests fix flake8 fix ci
| clf2.decision_function([[2., 2.]])) | ||
|
|
||
|
|
||
| # mock get_chunk_n_rows to actually test more than one chunk (here one |
There was a problem hiding this comment.
any idea how to merge these 2 tests ? they are the same, just with a different mocked chunk size
There was a problem hiding this comment.
Use pytest's monkeypatch fixture rather than unittest.mock alone?
But for this to be a reasonable test, you also need to test that the Mock has been called (or does patch do that?)
|
@agramfort this is ready for reviews |
sklearn/ensemble/iforest.py
Outdated
| " be removed in 0.22.", DeprecationWarning) | ||
| return self._threshold_ | ||
|
|
||
| def _compute_chunked_score_samples(self, X, working_memory=None): |
There was a problem hiding this comment.
If not for testing, why do we need the working_melody parameter here?
There was a problem hiding this comment.
Why do we need the working_memory parameter here?
There was a problem hiding this comment.
it is not useful indeed, thanks
| clf2.decision_function([[2., 2.]])) | ||
|
|
||
|
|
||
| # mock get_chunk_n_rows to actually test more than one chunk (here one |
There was a problem hiding this comment.
Use pytest's monkeypatch fixture rather than unittest.mock alone?
But for this to be a reasonable test, you also need to test that the Mock has been called (or does patch do that?)
|
Tests failing |
sklearn/ensemble/iforest.py
Outdated
| " be removed in 0.22.", DeprecationWarning) | ||
| return self._threshold_ | ||
|
|
||
| def _compute_chunked_score_samples(self, X, working_memory=None): |
There was a problem hiding this comment.
Why do we need the working_memory parameter here?
doc/whats_new/v0.21.rst
Outdated
| `Nicolas Goix`_. | ||
|
|
||
| - |Efficiency| :class:`ensemble.IsolationForest` now uses chunks of data at | ||
| prediction step. :issue:`13283` by `Nicolas Goix`_. |
There was a problem hiding this comment.
Perhaps say something like "capping the memory usage"
|
THanks @ngoix! |
…utation (scikit-learn#13283)" This reverts commit bce9351.
…utation (scikit-learn#13283)" This reverts commit bce9351.
fixes #12040
is currently based on top of #13260 (so do not review yet)
needs to be rebased on #13251