Skip to content

Commit

Permalink
Disallow multiple roots for tree_method=hist (dmlc#1979)
Browse files Browse the repository at this point in the history
As discussed in issue dmlc#1978, tree_method=hist ignores the parameter
param.num_roots; it simply assumes that the tree has only one root. In
particular, when InitData() method initializes row_set_collection_, it simply
assigns all rows to node 0, the value that's hard-coded.

For now, the updater will simply fail when num_roots exceeds 1. I will revise
the updater soon to support multiple roots.
  • Loading branch information
hcho3 authored and tqchen committed Jan 21, 2017
1 parent 036ee55 commit 5d74578
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 0 deletions.
1 change: 1 addition & 0 deletions src/tree/tree_updater.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ DMLC_REGISTRY_LINK_TAG(updater_colmaker);
DMLC_REGISTRY_LINK_TAG(updater_skmaker);
DMLC_REGISTRY_LINK_TAG(updater_refresh);
DMLC_REGISTRY_LINK_TAG(updater_prune);
DMLC_REGISTRY_LINK_TAG(updater_fast_hist);
DMLC_REGISTRY_LINK_TAG(updater_histmaker);
DMLC_REGISTRY_LINK_TAG(updater_sync);
} // namespace tree
Expand Down
3 changes: 3 additions & 0 deletions src/tree/updater_fast_hist.cc
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ class FastHistMaker: public TreeUpdater {
tstart = dmlc::GetTime();
this->InitData(gmat, gpair, *p_fmat, *p_tree);
time_init_data = dmlc::GetTime() - tstart;
// FIXME(hcho3): this code is broken when param.num_roots > 1. Please fix it
CHECK_EQ(p_tree->param.num_roots, 1)
<< "tree_method=hist does not support multiple roots at this moment";
for (int nid = 0; nid < p_tree->param.num_roots; ++nid) {
tstart = dmlc::GetTime();
hist_.AddHistRow(nid);
Expand Down

0 comments on commit 5d74578

Please sign in to comment.