From 0f9ffcdc16775d36b30accfe630aaaffca360465 Mon Sep 17 00:00:00 2001 From: Jiaming Yuan Date: Fri, 19 Nov 2021 21:40:04 +0800 Subject: [PATCH] [backport] Fix R CRAN failures. (#7404) (#7451) * Remove hist builder dtor. * Initialize values. * Tolerance. * Remove the use of nthread in col maker. --- R-package/tests/testthat/test_helpers.R | 2 +- include/xgboost/data.h | 2 +- src/data/simple_dmatrix.h | 8 ++++---- src/tree/updater_colmaker.cc | 27 +++++++++++++------------ src/tree/updater_quantile_hist.cc | 3 --- src/tree/updater_quantile_hist.h | 1 - 6 files changed, 20 insertions(+), 23 deletions(-) diff --git a/R-package/tests/testthat/test_helpers.R b/R-package/tests/testthat/test_helpers.R index d66b72430626..c426c8377468 100644 --- a/R-package/tests/testthat/test_helpers.R +++ b/R-package/tests/testthat/test_helpers.R @@ -228,7 +228,7 @@ if (grepl('Windows', Sys.info()[['sysname']]) || X <- 10^runif(100, -20, 20) if (capabilities('long.double')) { X2X <- as.numeric(format(X, digits = 17)) - expect_identical(X, X2X) + expect_equal(X, X2X, tolerance = float_tolerance) } # retrieved attributes to be the same as written for (x in X) { diff --git a/include/xgboost/data.h b/include/xgboost/data.h index cd000e371332..00cbb4be5283 100644 --- a/include/xgboost/data.h +++ b/include/xgboost/data.h @@ -211,7 +211,7 @@ struct Entry { */ struct BatchParam { /*! \brief The GPU device to use. */ - int gpu_id; + int gpu_id {-1}; /*! \brief Maximum number of bins per feature for histograms. */ int max_bin{0}; /*! \brief Hessian, used for sketching with future approx implementation. */ diff --git a/src/data/simple_dmatrix.h b/src/data/simple_dmatrix.h index 3529f1de4cc4..a4993e3b3827 100644 --- a/src/data/simple_dmatrix.h +++ b/src/data/simple_dmatrix.h @@ -49,10 +49,10 @@ class SimpleDMatrix : public DMatrix { MetaInfo info_; // Primary storage type std::shared_ptr sparse_page_ = std::make_shared(); - std::shared_ptr column_page_; - std::shared_ptr sorted_column_page_; - std::shared_ptr ellpack_page_; - std::shared_ptr gradient_index_; + std::shared_ptr column_page_{nullptr}; + std::shared_ptr sorted_column_page_{nullptr}; + std::shared_ptr ellpack_page_{nullptr}; + std::shared_ptr gradient_index_{nullptr}; BatchParam batch_param_; bool EllpackExists() const override { diff --git a/src/tree/updater_colmaker.cc b/src/tree/updater_colmaker.cc index 952a60f0fcb1..e981b76a8d25 100644 --- a/src/tree/updater_colmaker.cc +++ b/src/tree/updater_colmaker.cc @@ -109,10 +109,9 @@ class ColMaker: public TreeUpdater { interaction_constraints_.Configure(param_, dmat->Info().num_row_); // build tree for (auto tree : trees) { - Builder builder( - param_, - colmaker_param_, - interaction_constraints_, column_densities_); + CHECK(tparam_); + Builder builder(param_, colmaker_param_, interaction_constraints_, tparam_, + column_densities_); builder.Update(gpair->ConstHostVector(), dmat, tree); } param_.learning_rate = lr; @@ -154,12 +153,12 @@ class ColMaker: public TreeUpdater { class Builder { public: // constructor - explicit Builder(const TrainParam& param, - const ColMakerTrainParam& colmaker_train_param, + explicit Builder(const TrainParam ¶m, const ColMakerTrainParam &colmaker_train_param, FeatureInteractionConstraintHost _interaction_constraints, - const std::vector &column_densities) - : param_(param), colmaker_train_param_{colmaker_train_param}, - nthread_(omp_get_max_threads()), + GenericParameter const *ctx, const std::vector &column_densities) + : param_(param), + colmaker_train_param_{colmaker_train_param}, + ctx_{ctx}, tree_evaluator_(param_, column_densities.size(), GenericParameter::kCpuId), interaction_constraints_{std::move(_interaction_constraints)}, column_densities_(column_densities) {} @@ -238,7 +237,7 @@ class ColMaker: public TreeUpdater { // setup temp space for each thread // reserve a small space stemp_.clear(); - stemp_.resize(this->nthread_, std::vector()); + stemp_.resize(this->ctx_->Threads(), std::vector()); for (auto& i : stemp_) { i.clear(); i.reserve(256); } @@ -451,8 +450,9 @@ class ColMaker: public TreeUpdater { // start enumeration const auto num_features = static_cast(feat_set.size()); #if defined(_OPENMP) + CHECK(this->ctx_); const int batch_size = // NOLINT - std::max(static_cast(num_features / this->nthread_ / 32), 1); + std::max(static_cast(num_features / this->ctx_->Threads() / 32), 1); #endif // defined(_OPENMP) { auto page = batch.GetView(); @@ -553,7 +553,8 @@ class ColMaker: public TreeUpdater { virtual void SyncBestSolution(const std::vector &qexpand) { for (int nid : qexpand) { NodeEntry &e = snode_[nid]; - for (int tid = 0; tid < this->nthread_; ++tid) { + CHECK(this->ctx_); + for (int tid = 0; tid < this->ctx_->Threads(); ++tid) { e.best.Update(stemp_[tid][nid].best); } } @@ -609,7 +610,7 @@ class ColMaker: public TreeUpdater { const TrainParam& param_; const ColMakerTrainParam& colmaker_train_param_; // number of omp thread used during training - const int nthread_; + GenericParameter const* ctx_; common::ColumnSampler column_sampler_; // Instance Data: current node position in the tree of each instance std::vector position_; diff --git a/src/tree/updater_quantile_hist.cc b/src/tree/updater_quantile_hist.cc index bc894b4646b6..a5c91e085e5e 100644 --- a/src/tree/updater_quantile_hist.cc +++ b/src/tree/updater_quantile_hist.cc @@ -115,9 +115,6 @@ bool QuantileHistMaker::UpdatePredictionCache( } } -template -QuantileHistMaker::Builder::~Builder() = default; - template template diff --git a/src/tree/updater_quantile_hist.h b/src/tree/updater_quantile_hist.h index 69e42b90db44..5ce429c835b4 100644 --- a/src/tree/updater_quantile_hist.h +++ b/src/tree/updater_quantile_hist.h @@ -204,7 +204,6 @@ class QuantileHistMaker: public TreeUpdater { new HistogramBuilder} { builder_monitor_.Init("Quantile::Builder"); } - ~Builder(); // update one tree, growing virtual void Update(const GHistIndexMatrix& gmat, const ColumnMatrix& column_matrix,