Skip to content

Conversation

pprett
Copy link
Member

@pprett pprett commented Jul 22, 2013

This PR adresses issue #1802 by @yanirs.

It sets oob_score_ deprecated and introduces oob_improvement_ which gives the relative improvement of adding the i-th tree on the out-of-bag examples. The PR includes an example that shows how oob_improvement_ can be used to estimate the "optimal" number of iterations (basically an alternative to cross-validation).

gbrt_oob_example

@amueller
Copy link
Member

Is this what the R package does? It makes sense to me.
Tests are missing, though ;)

@pprett
Copy link
Member Author

pprett commented Jul 22, 2013

yes (but R's gbm also smooths the OOB improvements using LOESS)

2013/7/22 Andreas Mueller notifications@github.com

Is this what the R package does? It makes sense to me.
Tests are missing, though ;)


Reply to this email directly or view it on GitHubhttps://github.com//pull/2188#issuecomment-21358376
.

Peter Prettenhofer

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pl is the standard :)

@amueller
Copy link
Member

I like the example but I feel it is a bit hard to read. Could you add some comments please?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

n => n_samples?

@pprett
Copy link
Member Author

pprett commented Jul 23, 2013

@arjoly addressed - thx

@pprett
Copy link
Member Author

pprett commented Jul 23, 2013

@amueller I hope the example is clearer now

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nitpicking) maybe import directly GradientBoostingClassifier

@arjoly
Copy link
Member

arjoly commented Jul 24, 2013

I would add a test to check that oob_improvement_ is related to _oob_score_.
Furthermore you can think about adding a regression test.

Otherwise LGTM.

@pprett
Copy link
Member Author

pprett commented Jul 24, 2013

@arjoly I've added two regression tests and updated the narrative docs

@pprett
Copy link
Member Author

pprett commented Jul 24, 2013

@amueller @arjoly I think I've adressed the points you raised - it would be great if you could check - I'll then merge if its ok for you

@amueller
Copy link
Member

Thanks for addressing the comments. 👍 for merge.

@arjoly
Copy link
Member

arjoly commented Jul 24, 2013

LGTM!

@pprett pprett merged commit 63c8848 into scikit-learn:master Jul 24, 2013
@pprett
Copy link
Member Author

pprett commented Jul 24, 2013

merged - thanks for the reviews!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants