Fix test failure

dmlc · Dec 10, 2018 · 690ce0c · 690ce0c
1 parent ae0f5c9
commit 690ce0c
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 21 deletions.
diff --git a/src/tree/tree_model.cc b/src/tree/tree_model.cc
@@ -429,17 +429,17 @@ void DenseFeatureVector::Init(size_t size) {
   std::fill(data_.begin(), data_.end(), e);
 }
 
-void DenseFeatureVector::Fill(const SparsePage::Inst& inst) {
-  for (bst_uint i = 0; i < inst.size(); ++i) {
-    if (inst[i].index >= data_.size()) continue;
-    data_[inst[i].index].fvalue = inst[i].fvalue;
+void DenseFeatureVector::Fill(const SparsePage::Inst &inst) {
+  for (const auto &elem : inst) {
+    if (elem.index >= data_.size()) continue;
+    data_[elem.index].fvalue = elem.fvalue;
   }
 }
 
-void DenseFeatureVector::Drop(const SparsePage::Inst& inst) {
-  for (bst_uint i = 0; i < inst.size(); ++i) {
-    if (inst[i].index >= data_.size()) continue;
-    data_[inst[i].index].flag = -1;
+void DenseFeatureVector::Drop(const SparsePage::Inst &inst) {
+  for (const auto &elem : inst) {
+    if (elem.index >= data_.size()) continue;
+    data_[elem.index].flag = -1;
   }
 }
 

diff --git a/src/tree/updater_basemaker-inl.h b/src/tree/updater_basemaker-inl.h
@@ -41,7 +41,7 @@ class BaseMaker: public TreeUpdater {
     /*! \brief find type of each feature, use column format */
     inline void InitByCol(DMatrix* p_fmat,
                           const RegressionTree& tree) {
-      fminmax_.resize(tree.param.num_feature * 2);
+      fminmax_.resize(p_fmat->Info().num_col_ * 2);
       std::fill(fminmax_.begin(), fminmax_.end(),
                 -std::numeric_limits<bst_float>::max());
       // start accumulating statistics

diff --git a/src/tree/updater_histmaker.cc b/src/tree/updater_histmaker.cc
@@ -160,7 +160,7 @@ class HistMaker: public BaseMaker {
   virtual void InitWorkSet(DMatrix *p_fmat,
                            const RegressionTree &tree,
                            std::vector<bst_uint> *p_fset) {
-    p_fset->resize(tree.param.num_feature);
+    p_fset->resize(p_fmat->Info().num_col_);
     for (size_t i = 0; i < p_fset->size(); ++i) {
       (*p_fset)[i] = static_cast<unsigned>(i);
     }
@@ -327,7 +327,7 @@ class CQHistMaker: public HistMaker<TStats> {
                   const RegressionTree &tree) override {
     const MetaInfo &info = p_fmat->Info();
     // fill in reverse map
-    feat2workindex_.resize(tree.param.num_feature);
+    feat2workindex_.resize(p_fmat->Info().num_col_);
     std::fill(feat2workindex_.begin(), feat2workindex_.end(), -1);
     for (size_t i = 0; i < fset.size(); ++i) {
       feat2workindex_[fset[i]] = static_cast<int>(i);
@@ -386,7 +386,7 @@ class CQHistMaker: public HistMaker<TStats> {
                           const RegressionTree &tree) override {
     const MetaInfo &info = p_fmat->Info();
     // fill in reverse map
-    feat2workindex_.resize(tree.param.num_feature);
+    feat2workindex_.resize(p_fmat->Info().num_col_);
     std::fill(feat2workindex_.begin(), feat2workindex_.end(), -1);
     work_set_.clear();
     for (auto fidx : fset) {
@@ -685,7 +685,7 @@ class GlobalProposalHistMaker: public CQHistMaker<TStats> {
                   const RegressionTree &tree) override {
     const MetaInfo &info = p_fmat->Info();
     // fill in reverse map
-    this->feat2workindex_.resize(tree.param.num_feature);
+    this->feat2workindex_.resize(p_fmat->Info().num_col_);
     this->work_set_ = fset;
     std::fill(this->feat2workindex_.begin(), this->feat2workindex_.end(), -1);
     for (size_t i = 0; i < fset.size(); ++i) {

diff --git a/src/tree/updater_refresh.cc b/src/tree/updater_refresh.cc
@@ -48,7 +48,7 @@ class TreeRefresher: public TreeUpdater {
       }
       stemp[tid].resize(num_nodes, TStats(param_));
       std::fill(stemp[tid].begin(), stemp[tid].end(), TStats(param_));
-      fvec_temp[tid].Init(trees[0]->param.num_feature);
+      fvec_temp[tid].Init(p_fmat->Info().num_col_);
     }
     // if it is C++11, use lazy evaluation for Allreduce,
     // to gain speedup in recovery

diff --git a/src/tree/updater_skmaker.cc b/src/tree/updater_skmaker.cc
@@ -133,7 +133,7 @@ class SketchMaker: public BaseMaker {
                           DMatrix *p_fmat,
                           const RegressionTree &tree) {
     const MetaInfo& info = p_fmat->Info();
-    sketchs_.resize(this->qexpand_.size() * tree.param.num_feature * 3);
+    sketchs_.resize(this->qexpand_.size() * p_fmat->Info().num_col_ * 3);
     for (auto & sketch : sketchs_) {
       sketch.Init(info.num_row_, this->param_.sketch_eps);
     }
@@ -146,7 +146,7 @@ class SketchMaker: public BaseMaker {
       const auto nsize = static_cast<bst_omp_uint>(batch.Size());
       #pragma omp parallel for schedule(dynamic, 1)
       for (bst_omp_uint fidx = 0; fidx < nsize; ++fidx) {
-        this->UpdateSketchCol(gpair, batch[fidx], tree,
+        this->UpdateSketchCol(p_fmat, gpair, batch[fidx], tree,
                               node_stats_,
                               fidx,
                               batch[fidx].size() == nrows,
@@ -167,7 +167,7 @@ class SketchMaker: public BaseMaker {
     sketch_reducer_.Allreduce(dmlc::BeginPtr(summary_array_), nbytes, summary_array_.size());
   }
   // update sketch information in column fid
-  inline void UpdateSketchCol(const std::vector<GradientPair> &gpair,
+  inline void UpdateSketchCol(DMatrix *p_fmat, const std::vector<GradientPair> &gpair,
                               const SparsePage::Inst &col,
                               const RegressionTree &tree,
                               const std::vector<SKStats> &nstats,
@@ -182,7 +182,7 @@ class SketchMaker: public BaseMaker {
       const unsigned wid = this->node2workindex_[nid];
       for (int k = 0; k < 3; ++k) {
         sbuilder[3 * nid + k].sum_total = 0.0f;
-        sbuilder[3 * nid + k].sketch = &sketchs_[(wid * tree.param.num_feature + fid) * 3 + k];
+        sbuilder[3 * nid + k].sketch = &sketchs_[(wid * p_fmat->Info().num_col_ + fid) * 3 + k];
       }
     }
     if (!col_full) {
@@ -259,7 +259,7 @@ class SketchMaker: public BaseMaker {
                         const std::vector<GradientPair> &gpair,
                         DMatrix *p_fmat,
                         RegressionTree *p_tree) {
-    const bst_uint num_feature = p_tree->param.num_feature;
+    const bst_uint num_feature = p_fmat->Info().num_col_;
     // get the best split condition for each node
     std::vector<SplitEntry> sol(qexpand_.size());
     auto nexpand = static_cast<bst_omp_uint>(qexpand_.size());
@@ -269,7 +269,7 @@ class SketchMaker: public BaseMaker {
       CHECK_EQ(node2workindex_[nid], static_cast<int>(wid));
       SplitEntry &best = sol[wid];
       for (bst_uint fid = 0; fid < num_feature; ++fid) {
-        unsigned base = (wid * p_tree->param.num_feature + fid) * 3;
+        unsigned base = (wid * p_fmat->Info().num_col_ + fid) * 3;
         EnumerateSplit(summary_array_[base + 0],
                        summary_array_[base + 1],
                        summary_array_[base + 2],

diff --git a/tests/cpp/tree/test_refresh.cc b/tests/cpp/tree/test_refresh.cc
@@ -25,7 +25,6 @@ TEST(Updater, Refresh) {
     {"reg_lambda", "1"}};
 
   RegressionTree tree = RegressionTree();
-  tree.param.InitAllowUnknown(cfg);
   std::vector<RegressionTree*> trees {&tree};
   std::unique_ptr<TreeUpdater> refresher(TreeUpdater::Create("refresh"));