Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MRG] avoid storage of each tree predictions in iforest #13260

Merged
merged 1 commit into from Mar 2, 2019

Conversation

7 participants
@ngoix
Copy link
Contributor

commented Feb 25, 2019

fixes (partly) #12040

will do the chunking of X_test in another PR for clarity

@ngoix

This comment has been minimized.

Copy link
Contributor Author

commented Feb 25, 2019

should not be too hard to rebase on #13251

@ngoix ngoix changed the title [WIP] avoid storage of each tree predictions [MRG] avoid storage of each tree predictions Feb 26, 2019

@ngoix ngoix changed the title [MRG] avoid storage of each tree predictions [MRG] avoid storage of each tree predictions in iforest Feb 26, 2019

@glemaitre

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2019

It seems that you are modifying something which is changed in #13251 as well.
@albertcthomas it seems that we should kill one of the 2 PRs isn't it?

@glemaitre
Copy link
Contributor

left a comment

Actually I see that we could use the common tests from @albertcthomas

Show resolved Hide resolved sklearn/ensemble/iforest.py
@ngoix

This comment has been minimized.

Copy link
Contributor Author

commented Feb 26, 2019

I don't think so, #13251 has nothing to do with the changes here. Conflicts will be on the changes in value of _average_path_length(1) and _average_path_length(2) so shouldn't be a problem. Let's merge #13251 first.

@albertcthomas

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2019

I think @ngoix is right, #13251 only changes a few output values of _average_path_length.

@albertcthomas

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2019

#13251 has just been merged so you can rebase @ngoix.

Show resolved Hide resolved sklearn/ensemble/iforest.py Outdated
Show resolved Hide resolved sklearn/ensemble/iforest.py

@agramfort agramfort added this to In progress in Sprint Paris 2019 Feb 28, 2019

@albertcthomas
Copy link
Contributor

left a comment

LGTM!

Show resolved Hide resolved sklearn/ensemble/iforest.py

@jnothman jnothman moved this from In progress to Needs review in Sprint Paris 2019 Feb 28, 2019

@agramfort

This comment has been minimized.

Copy link
Member

commented Feb 28, 2019

@ngoix please update what's new 0.21.rst

and +1 for MRG. thanks

@ngoix ngoix force-pushed the ngoix:iforest_memory branch from b0de10a to 600a158 Mar 1, 2019

@banilo

This comment has been minimized.

Copy link
Contributor

commented Mar 1, 2019

looks good

@ngoix

This comment has been minimized.

Copy link
Contributor Author

commented Mar 2, 2019

@agramfort can you merge?

@adrinjalali
Copy link
Member

left a comment

LGTM, thanks!

@adrinjalali adrinjalali merged commit 12705bb into scikit-learn:master Mar 2, 2019

16 checks passed

LGTM analysis: C/C++ No code changes detected
Details
LGTM analysis: JavaScript No code changes detected
Details
LGTM analysis: Python 1 new alert
Details
ci/circleci: deploy Your tests passed on CircleCI!
Details
ci/circleci: doc Your tests passed on CircleCI!
Details
ci/circleci: doc-min-dependencies Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 92.37%)
Details
codecov/project Absolute coverage decreased by -<.01% but relative coverage increased by +7.62% compared to 984871b
Details
scikit-learn.scikit-learn Build #20190301.76 succeeded
Details
scikit-learn.scikit-learn (Linux py35_conda_openblas) Linux py35_conda_openblas succeeded
Details
scikit-learn.scikit-learn (Linux py35_np_atlas) Linux py35_np_atlas succeeded
Details
scikit-learn.scikit-learn (Linux pylatest_conda) Linux pylatest_conda succeeded
Details
scikit-learn.scikit-learn (Windows py35_32) Windows py35_32 succeeded
Details
scikit-learn.scikit-learn (Windows py37_64) Windows py37_64 succeeded
Details
scikit-learn.scikit-learn (macOS pylatest_conda) macOS pylatest_conda succeeded
Details

Sprint Paris 2019 automation moved this from Needs review to Done Mar 2, 2019

xhlulu added a commit to xhlulu/scikit-learn that referenced this pull request Apr 28, 2019

xhlulu added a commit to xhlulu/scikit-learn that referenced this pull request Apr 28, 2019

xhlulu added a commit to xhlulu/scikit-learn that referenced this pull request Apr 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.