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

support max_depth option, to solve (#33) #35

Merged
merged 1 commit into from
Oct 25, 2016
Merged

support max_depth option, to solve (#33) #35

merged 1 commit into from
Oct 25, 2016

Conversation

guolinke
Copy link
Collaborator

@guolinke guolinke commented Oct 24, 2016

To solve #33

@guolinke guolinke changed the title support max_depth option, to solve #33 support max_depth option, to solve (#33) Oct 24, 2016
@@ -139,6 +142,8 @@ class Tree {
int* leaf_parent_;
/*! \brief Output of leaves */
score_t* leaf_value_;
/*! \brief Depth for leaves */
int* leaf_depth_;
Copy link
Collaborator

Choose a reason for hiding this comment

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

is there a need for every leaves ?

Copy link
Collaborator Author

@guolinke guolinke Oct 25, 2016

Choose a reason for hiding this comment

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

Yes, since we don't know which leaf will be splitted ..

@@ -163,6 +163,8 @@ class SerialTreeLearner: public TreeLearner {
double histogram_pool_size_;
/*! \brief used to cache historical histogram to speed up*/
LRUPool<FeatureHistogram*> histogram_pool_;
/*! \brief max depth of tree model */
int max_depth_;
Copy link
Collaborator

Choose a reason for hiding this comment

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

i think it will be good to make max_depth as an early_stop condition

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think early stopping is for the progress of boosting, not for tree growth..

@chivee chivee merged commit a6a75fe into microsoft:master Oct 25, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Mar 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants