From 0012f2ef93f2fbe526227f8b0e8d792073bc3722 Mon Sep 17 00:00:00 2001 From: Jiaming Yuan Date: Sun, 5 Apr 2020 04:42:29 +0800 Subject: [PATCH] Upgrade clang-tidy on CI. (#5469) * Correct all clang-tidy errors. * Upgrade clang-tidy to 10 on CI. Co-authored-by: Hyunsu Cho --- .clang-tidy | 38 ++-- Jenkinsfile | 2 +- include/xgboost/base.h | 2 +- include/xgboost/data.h | 26 +-- include/xgboost/feature_map.h | 2 +- include/xgboost/generic_parameters.h | 2 +- include/xgboost/host_device_vector.h | 2 +- include/xgboost/json.h | 155 ++++++++-------- include/xgboost/json_io.h | 5 +- include/xgboost/learner.h | 8 +- include/xgboost/linear_updater.h | 2 +- include/xgboost/metric.h | 6 +- include/xgboost/objective.h | 2 +- include/xgboost/predictor.h | 4 +- include/xgboost/span.h | 22 +-- include/xgboost/tree_updater.h | 2 +- plugin/example/custom_obj.cc | 4 +- src/common/bitfield.h | 14 +- src/common/column_matrix.h | 30 ++-- src/common/common.h | 2 +- src/common/compressed_iterator.h | 12 +- src/common/config.h | 10 +- src/common/device_helpers.cu | 16 +- src/common/device_helpers.cuh | 167 +++++++++--------- src/common/hist_util.cc | 64 +++---- src/common/hist_util.h | 79 +++++---- src/common/json.cc | 58 +++--- src/common/observer.h | 20 +-- src/common/probability_distribution.h | 1 + src/common/row_set.h | 17 +- src/common/timer.cc | 12 +- src/common/timer.cu | 4 +- src/common/timer.h | 12 +- src/common/transform.h | 3 +- src/data/array_interface.h | 5 +- src/data/device_adapter.cuh | 36 ++-- src/data/device_dmatrix.cu | 12 +- src/data/device_dmatrix.h | 6 +- src/data/ellpack_page.cuh | 8 +- src/data/ellpack_page_raw_format.cu | 12 +- src/data/simple_dmatrix.cc | 36 ++-- src/data/simple_dmatrix.cu | 10 +- src/data/simple_dmatrix.h | 2 +- src/data/sparse_page_source.h | 6 +- src/gbm/gblinear.cc | 30 ++-- src/gbm/gblinear_model.h | 44 ++--- src/gbm/gbtree.cc | 36 ++-- src/gbm/gbtree.h | 4 +- src/gbm/gbtree_model.cc | 4 +- src/gbm/gbtree_model.h | 6 +- src/learner.cc | 8 +- src/linear/coordinate_common.h | 26 +-- src/linear/updater_coordinate.cc | 16 +- src/linear/updater_gpu_coordinate.cu | 30 ++-- src/linear/updater_shotgun.cc | 8 +- src/metric/elementwise_metric.cu | 5 +- src/metric/metric_common.h | 6 +- src/metric/rank_metric.cu | 1 - src/metric/survival_metric.cc | 4 +- src/objective/aft_obj.cc | 4 +- src/objective/multiclass_obj.cu | 4 +- src/objective/rank_obj.cu | 14 +- src/objective/regression_obj.cu | 12 +- src/predictor/cpu_predictor.cc | 54 +++--- src/predictor/gpu_predictor.cu | 17 +- src/tree/constraints.cu | 2 +- src/tree/constraints.h | 2 - src/tree/gpu_hist/gradient_based_sampler.cu | 7 +- src/tree/gpu_hist/row_partitioner.cu | 67 +++---- src/tree/gpu_hist/row_partitioner.cuh | 64 +++---- src/tree/param.h | 6 +- src/tree/tree_model.cc | 8 +- src/tree/updater_basemaker-inl.h | 4 +- src/tree/updater_colmaker.cc | 18 +- src/tree/updater_gpu_common.cuh | 39 ++-- src/tree/updater_gpu_hist.cu | 28 +-- src/tree/updater_prune.cc | 4 +- src/tree/updater_quantile_hist.cc | 8 +- src/tree/updater_quantile_hist.h | 9 +- src/tree/updater_refresh.cc | 4 +- src/tree/updater_skmaker.cc | 8 +- tests/ci_build/Dockerfile.clang_tidy | 11 +- tests/ci_build/tidy.py | 47 +++-- tests/cpp/c_api/test_c_api.cc | 10 +- tests/cpp/common/test_bitfield.cu | 4 +- tests/cpp/common/test_column_matrix.cc | 20 +-- tests/cpp/common/test_device_helpers.cu | 8 +- tests/cpp/common/test_group_data.cc | 2 +- tests/cpp/common/test_hist_util.cc | 50 +++--- tests/cpp/common/test_hist_util.cu | 30 ++-- tests/cpp/common/test_host_device_vector.cu | 2 +- tests/cpp/common/test_json.cc | 4 +- tests/cpp/common/test_transform_range.cu | 2 +- tests/cpp/data/test_adapter.cc | 2 +- tests/cpp/data/test_device_adapter.cu | 2 +- tests/cpp/data/test_device_dmatrix.cu | 8 +- tests/cpp/data/test_ellpack_page.cu | 6 +- tests/cpp/data/test_sparse_page_dmatrix.cu | 5 +- tests/cpp/linear/test_linear.cc | 4 +- tests/cpp/linear/test_linear.cu | 2 +- tests/cpp/predictor/test_gpu_predictor.cu | 2 +- tests/cpp/test_learner.cc | 2 +- tests/cpp/test_serialization.cc | 18 +- .../gpu_hist/test_gradient_based_sampler.cu | 6 +- tests/cpp/tree/test_constraints.cu | 25 +-- tests/cpp/tree/test_gpu_hist.cu | 6 +- tests/cpp/tree/test_quantile_hist.cc | 8 +- 107 files changed, 932 insertions(+), 903 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index cf3bcbffe..3be1d9e0c 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,21 +1,21 @@ Checks: 'modernize-*,-modernize-make-*,-modernize-use-auto,-modernize-raw-string-literal,-modernize-avoid-c-arrays,-modernize-use-trailing-return-type,google-*,-google-default-arguments,-clang-diagnostic-#pragma-messages,readability-identifier-naming' CheckOptions: - - { key: readability-identifier-naming.ClassCase, value: CamelCase } - - { key: readability-identifier-naming.StructCase, value: CamelCase } - - { key: readability-identifier-naming.TypeAliasCase, value: CamelCase } - - { key: readability-identifier-naming.TypedefCase, value: CamelCase } - - { key: readability-identifier-naming.TypeTemplateParameterCase, value: CamelCase } - - { key: readability-identifier-naming.MemberCase, value: lower_case } - - { key: readability-identifier-naming.PrivateMemberSuffix, value: '_' } - - { key: readability-identifier-naming.ProtectedMemberSuffix, value: '_' } - - { key: readability-identifier-naming.EnumCase, value: CamelCase } - - { key: readability-identifier-naming.EnumConstant, value: CamelCase } - - { key: readability-identifier-naming.EnumConstantPrefix, value: k } - - { key: readability-identifier-naming.GlobalConstantCase, value: CamelCase } - - { key: readability-identifier-naming.GlobalConstantPrefix, value: k } - - { key: readability-identifier-naming.StaticConstantCase, value: CamelCase } - - { key: readability-identifier-naming.StaticConstantPrefix, value: k } - - { key: readability-identifier-naming.ConstexprVariableCase, value: CamelCase } - - { key: readability-identifier-naming.ConstexprVariablePrefix, value: k } - - { key: readability-identifier-naming.FunctionCase, value: CamelCase } - - { key: readability-identifier-naming.NamespaceCase, value: lower_case } + - { key: readability-identifier-naming.ClassCase, value: CamelCase } + - { key: readability-identifier-naming.StructCase, value: CamelCase } + - { key: readability-identifier-naming.TypeAliasCase, value: CamelCase } + - { key: readability-identifier-naming.TypedefCase, value: CamelCase } + - { key: readability-identifier-naming.TypeTemplateParameterCase, value: CamelCase } + - { key: readability-identifier-naming.MemberCase, value: lower_case } + - { key: readability-identifier-naming.PrivateMemberSuffix, value: '_' } + - { key: readability-identifier-naming.ProtectedMemberSuffix, value: '_' } + - { key: readability-identifier-naming.EnumCase, value: CamelCase } + - { key: readability-identifier-naming.EnumConstant, value: CamelCase } + - { key: readability-identifier-naming.EnumConstantPrefix, value: k } + - { key: readability-identifier-naming.GlobalConstantCase, value: CamelCase } + - { key: readability-identifier-naming.GlobalConstantPrefix, value: k } + - { key: readability-identifier-naming.StaticConstantCase, value: CamelCase } + - { key: readability-identifier-naming.StaticConstantPrefix, value: k } + - { key: readability-identifier-naming.ConstexprVariableCase, value: CamelCase } + - { key: readability-identifier-naming.ConstexprVariablePrefix, value: k } + - { key: readability-identifier-naming.FunctionCase, value: CamelCase } + - { key: readability-identifier-naming.NamespaceCase, value: lower_case } diff --git a/Jenkinsfile b/Jenkinsfile index 1128f7dfe..ed594fb8d 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -119,7 +119,7 @@ def ClangTidy() { echo "Running clang-tidy job..." def container_type = "clang_tidy" def docker_binary = "docker" - def dockerArgs = "--build-arg CUDA_VERSION=9.2" + def dockerArgs = "--build-arg CUDA_VERSION=10.1" sh """ ${dockerRun} ${container_type} ${docker_binary} ${dockerArgs} python3 tests/ci_build/tidy.py """ diff --git a/include/xgboost/base.h b/include/xgboost/base.h index 52ed13d29..32af1077b 100644 --- a/include/xgboost/base.h +++ b/include/xgboost/base.h @@ -106,7 +106,7 @@ using bst_uint = uint32_t; // NOLINT /*! \brief integer type. */ using bst_int = int32_t; // NOLINT /*! \brief unsigned long integers */ -using bst_ulong = uint64_t; +using bst_ulong = uint64_t; // NOLINT /*! \brief float type, used for storing statistics */ using bst_float = float; // NOLINT diff --git a/include/xgboost/data.h b/include/xgboost/data.h index f07dd11f1..c66f60aff 100644 --- a/include/xgboost/data.h +++ b/include/xgboost/data.h @@ -42,34 +42,34 @@ class MetaInfo { static constexpr uint64_t kNumField = 9; /*! \brief number of rows in the data */ - uint64_t num_row_{0}; + uint64_t num_row_{0}; // NOLINT /*! \brief number of columns in the data */ - uint64_t num_col_{0}; + uint64_t num_col_{0}; // NOLINT /*! \brief number of nonzero entries in the data */ - uint64_t num_nonzero_{0}; + uint64_t num_nonzero_{0}; // NOLINT /*! \brief label of each instance */ - HostDeviceVector labels_; + HostDeviceVector labels_; // NOLINT /*! * \brief the index of begin and end of a group * needed when the learning task is ranking. */ - std::vector group_ptr_; + std::vector group_ptr_; // NOLINT /*! \brief weights of each instance, optional */ - HostDeviceVector weights_; + HostDeviceVector weights_; // NOLINT /*! * \brief initialized margins, * if specified, xgboost will start from this init margin * can be used to specify initial prediction to boost from. */ - HostDeviceVector base_margin_; + HostDeviceVector base_margin_; // NOLINT /*! * \brief lower bound of the label, to be used for survival analysis (censored regression) */ - HostDeviceVector labels_lower_bound_; + HostDeviceVector labels_lower_bound_; // NOLINT /*! * \brief upper bound of the label, to be used for survival analysis (censored regression) */ - HostDeviceVector labels_upper_bound_; + HostDeviceVector labels_upper_bound_; // NOLINT /*! \brief default constructor */ MetaInfo() = default; @@ -360,7 +360,7 @@ class BatchIteratorImpl { template class BatchIterator { public: - using iterator_category = std::forward_iterator_tag; + using iterator_category = std::forward_iterator_tag; // NOLINT explicit BatchIterator(BatchIteratorImpl* impl) { impl_.reset(impl); } void operator++() { @@ -395,9 +395,9 @@ class BatchIterator { template class BatchSet { public: - explicit BatchSet(BatchIterator begin_iter) : begin_iter_(begin_iter) {} - BatchIterator begin() { return begin_iter_; } - BatchIterator end() { return BatchIterator(nullptr); } + explicit BatchSet(BatchIterator begin_iter) : begin_iter_(std::move(begin_iter)) {} + BatchIterator begin() { return begin_iter_; } // NOLINT + BatchIterator end() { return BatchIterator(nullptr); } // NOLINT private: BatchIterator begin_iter_; diff --git a/include/xgboost/feature_map.h b/include/xgboost/feature_map.h index 56dd126a5..a48e28ba1 100644 --- a/include/xgboost/feature_map.h +++ b/include/xgboost/feature_map.h @@ -65,7 +65,7 @@ class FeatureMap { return names_[idx].c_str(); } /*! \return type of specific feature */ - Type type(size_t idx) const { + Type TypeOf(size_t idx) const { CHECK_LT(idx, names_.size()) << "FeatureMap feature index exceed bound"; return types_[idx]; } diff --git a/include/xgboost/generic_parameters.h b/include/xgboost/generic_parameters.h index 925af2d3c..be068f74a 100644 --- a/include/xgboost/generic_parameters.h +++ b/include/xgboost/generic_parameters.h @@ -89,7 +89,7 @@ struct GenericParameter : public XGBoostParameter { private: // number of devices to use (deprecated). - int n_gpus {0}; + int n_gpus {0}; // NOLINT }; } // namespace xgboost diff --git a/include/xgboost/host_device_vector.h b/include/xgboost/host_device_vector.h index b3ed36b99..75595187e 100644 --- a/include/xgboost/host_device_vector.h +++ b/include/xgboost/host_device_vector.h @@ -130,7 +130,7 @@ class HostDeviceVector { void Resize(size_t new_size, T v = T()); - using value_type = T; + using value_type = T; // NOLINT private: HostDeviceVectorImpl* impl_; diff --git a/include/xgboost/json.h b/include/xgboost/json.h index 13fde3b58..c190505bf 100644 --- a/include/xgboost/json.h +++ b/include/xgboost/json.h @@ -24,13 +24,13 @@ class Value { public: /*!\brief Simplified implementation of LLVM RTTI. */ enum class ValueKind { - String, - Number, - Integer, - Object, // std::map - Array, // std::vector - Boolean, - Null + kString, + kNumber, + kInteger, + kObject, // std::map + kArray, // std::vector + kBoolean, + kNull }; explicit Value(ValueKind _kind) : kind_{_kind} {} @@ -54,7 +54,7 @@ class Value { template bool IsA(Value const* value) { - return T::isClassOf(value); + return T::IsClassOf(value); } template @@ -70,26 +70,26 @@ T* Cast(U* value) { class JsonString : public Value { std::string str_; public: - JsonString() : Value(ValueKind::String) {} + JsonString() : Value(ValueKind::kString) {} JsonString(std::string const& str) : // NOLINT - Value(ValueKind::String), str_{str} {} + Value(ValueKind::kString), str_{str} {} JsonString(std::string&& str) : // NOLINT - Value(ValueKind::String), str_{std::move(str)} {} + Value(ValueKind::kString), str_{std::move(str)} {} void Save(JsonWriter* writer) override; Json& operator[](std::string const & key) override; Json& operator[](int ind) override; - std::string const& getString() && { return str_; } - std::string const& getString() const & { return str_; } - std::string& getString() & { return str_; } + std::string const& GetString() && { return str_; } + std::string const& GetString() const & { return str_; } + std::string& GetString() & { return str_; } bool operator==(Value const& rhs) const override; Value& operator=(Value const& rhs) override; - static bool isClassOf(Value const* value) { - return value->Type() == ValueKind::String; + static bool IsClassOf(Value const* value) { + return value->Type() == ValueKind::kString; } }; @@ -97,11 +97,11 @@ class JsonArray : public Value { std::vector vec_; public: - JsonArray() : Value(ValueKind::Array) {} + JsonArray() : Value(ValueKind::kArray) {} JsonArray(std::vector&& arr) : // NOLINT - Value(ValueKind::Array), vec_{std::move(arr)} {} + Value(ValueKind::kArray), vec_{std::move(arr)} {} JsonArray(std::vector const& arr) : // NOLINT - Value(ValueKind::Array), vec_{arr} {} + Value(ValueKind::kArray), vec_{arr} {} JsonArray(JsonArray const& that) = delete; JsonArray(JsonArray && that); @@ -110,15 +110,15 @@ class JsonArray : public Value { Json& operator[](std::string const & key) override; Json& operator[](int ind) override; - std::vector const& getArray() && { return vec_; } - std::vector const& getArray() const & { return vec_; } - std::vector& getArray() & { return vec_; } + std::vector const& GetArray() && { return vec_; } + std::vector const& GetArray() const & { return vec_; } + std::vector& GetArray() & { return vec_; } bool operator==(Value const& rhs) const override; Value& operator=(Value const& rhs) override; - static bool isClassOf(Value const* value) { - return value->Type() == ValueKind::Array; + static bool IsClassOf(Value const* value) { + return value->Type() == ValueKind::kArray; } }; @@ -126,7 +126,7 @@ class JsonObject : public Value { std::map object_; public: - JsonObject() : Value(ValueKind::Object) {} + JsonObject() : Value(ValueKind::kObject) {} JsonObject(std::map&& object); // NOLINT JsonObject(JsonObject const& that) = delete; JsonObject(JsonObject && that); @@ -136,17 +136,17 @@ class JsonObject : public Value { Json& operator[](std::string const & key) override; Json& operator[](int ind) override; - std::map const& getObject() && { return object_; } - std::map const& getObject() const & { return object_; } - std::map & getObject() & { return object_; } + std::map const& GetObject() && { return object_; } + std::map const& GetObject() const & { return object_; } + std::map & GetObject() & { return object_; } bool operator==(Value const& rhs) const override; Value& operator=(Value const& rhs) override; - static bool isClassOf(Value const* value) { - return value->Type() == ValueKind::Object; + static bool IsClassOf(Value const* value) { + return value->Type() == ValueKind::kObject; } - virtual ~JsonObject() = default; + ~JsonObject() override = default; }; class JsonNumber : public Value { @@ -154,18 +154,18 @@ class JsonNumber : public Value { using Float = float; private: - Float number_; + Float number_ { 0 }; public: - JsonNumber() : Value(ValueKind::Number) {} + JsonNumber() : Value(ValueKind::kNumber) {} template ::value>::type* = nullptr> - JsonNumber(FloatT value) : Value(ValueKind::Number) { // NOLINT + JsonNumber(FloatT value) : Value(ValueKind::kNumber) { // NOLINT number_ = value; } template ::value>::type* = nullptr> - JsonNumber(FloatT value) : Value{ValueKind::Number}, // NOLINT + JsonNumber(FloatT value) : Value{ValueKind::kNumber}, // NOLINT number_{static_cast(value)} {} void Save(JsonWriter* writer) override; @@ -173,16 +173,16 @@ class JsonNumber : public Value { Json& operator[](std::string const & key) override; Json& operator[](int ind) override; - Float const& getNumber() && { return number_; } - Float const& getNumber() const & { return number_; } - Float& getNumber() & { return number_; } + Float const& GetNumber() && { return number_; } + Float const& GetNumber() const & { return number_; } + Float& GetNumber() & { return number_; } bool operator==(Value const& rhs) const override; Value& operator=(Value const& rhs) override; - static bool isClassOf(Value const* value) { - return value->Type() == ValueKind::Number; + static bool IsClassOf(Value const* value) { + return value->Type() == ValueKind::kNumber; } }; @@ -191,27 +191,27 @@ class JsonInteger : public Value { using Int = int64_t; private: - Int integer_; + Int integer_ {0}; public: - JsonInteger() : Value(ValueKind::Integer), integer_{0} {} // NOLINT + JsonInteger() : Value(ValueKind::kInteger) {} // NOLINT template ::value>::type* = nullptr> - JsonInteger(IntT value) : Value(ValueKind::Integer), integer_{value} {} // NOLINT + JsonInteger(IntT value) : Value(ValueKind::kInteger), integer_{value} {} // NOLINT template ::value>::type* = nullptr> - JsonInteger(IntT value) : Value(ValueKind::Integer), // NOLINT + JsonInteger(IntT value) : Value(ValueKind::kInteger), // NOLINT integer_{static_cast(value)} {} template ::value>::type* = nullptr> - JsonInteger(IntT value) : Value(ValueKind::Integer), // NOLINT + JsonInteger(IntT value) : Value(ValueKind::kInteger), // NOLINT integer_{static_cast(value)} {} template ::value && !std::is_same::value>::type * = nullptr> JsonInteger(IntT value) // NOLINT - : Value(ValueKind::Integer), + : Value(ValueKind::kInteger), integer_{static_cast(value)} {} Json& operator[](std::string const & key) override; @@ -220,20 +220,20 @@ class JsonInteger : public Value { bool operator==(Value const& rhs) const override; Value& operator=(Value const& rhs) override; - Int const& getInteger() && { return integer_; } - Int const& getInteger() const & { return integer_; } - Int& getInteger() & { return integer_; } + Int const& GetInteger() && { return integer_; } + Int const& GetInteger() const & { return integer_; } + Int& GetInteger() & { return integer_; } void Save(JsonWriter* writer) override; - static bool isClassOf(Value const* value) { - return value->Type() == ValueKind::Integer; + static bool IsClassOf(Value const* value) { + return value->Type() == ValueKind::kInteger; } }; class JsonNull : public Value { public: - JsonNull() : Value(ValueKind::Null) {} - JsonNull(std::nullptr_t) : Value(ValueKind::Null) {} // NOLINT + JsonNull() : Value(ValueKind::kNull) {} + JsonNull(std::nullptr_t) : Value(ValueKind::kNull) {} // NOLINT void Save(JsonWriter* writer) override; @@ -243,8 +243,8 @@ class JsonNull : public Value { bool operator==(Value const& rhs) const override; Value& operator=(Value const& rhs) override; - static bool isClassOf(Value const* value) { - return value->Type() == ValueKind::Null; + static bool IsClassOf(Value const* value) { + return value->Type() == ValueKind::kNull; } }; @@ -253,33 +253,34 @@ class JsonBoolean : public Value { bool boolean_; public: - JsonBoolean() : Value(ValueKind::Boolean) {} // NOLINT + JsonBoolean() : Value(ValueKind::kBoolean) {} // NOLINT // Ambigious with JsonNumber. template ::value || std::is_same::value>::type* = nullptr> JsonBoolean(Bool value) : // NOLINT - Value(ValueKind::Boolean), boolean_{value} {} + Value(ValueKind::kBoolean), boolean_{value} {} void Save(JsonWriter* writer) override; Json& operator[](std::string const & key) override; Json& operator[](int ind) override; - bool const& getBoolean() && { return boolean_; } - bool const& getBoolean() const & { return boolean_; } - bool& getBoolean() & { return boolean_; } + bool const& GetBoolean() && { return boolean_; } + bool const& GetBoolean() const & { return boolean_; } + bool& GetBoolean() & { return boolean_; } bool operator==(Value const& rhs) const override; Value& operator=(Value const& rhs) override; - static bool isClassOf(Value const* value) { - return value->Type() == ValueKind::Boolean; + static bool IsClassOf(Value const* value) { + return value->Type() == ValueKind::kBoolean; } }; struct StringView { + private: using CharT = char; // unsigned char CharT const* str_; size_t size_; @@ -392,7 +393,7 @@ class Json { } // copy - Json(Json const& other) : ptr_{other.ptr_} {} + Json(Json const& other) = default; Json& operator=(Json const& other); // move Json(Json&& other) : ptr_{std::move(other.ptr_)} {} @@ -439,13 +440,13 @@ template ::value>::type* = nullptr> JsonNumber::Float& GetImpl(T& val) { // NOLINT - return val.getNumber(); + return val.GetNumber(); } template ::value>::type* = nullptr> JsonNumber::Float const& GetImpl(T& val) { // NOLINT - return val.getNumber(); + return val.GetNumber(); } // Integer @@ -453,13 +454,13 @@ template ::value>::type* = nullptr> JsonInteger::Int& GetImpl(T& val) { // NOLINT - return val.getInteger(); + return val.GetInteger(); } template ::value>::type* = nullptr> JsonInteger::Int const& GetImpl(T& val) { // NOLINT - return val.getInteger(); + return val.GetInteger(); } // String @@ -467,13 +468,13 @@ template ::value>::type* = nullptr> std::string& GetImpl(T& val) { // NOLINT - return val.getString(); + return val.GetString(); } template ::value>::type* = nullptr> std::string const& GetImpl(T& val) { // NOLINT - return val.getString(); + return val.GetString(); } // Boolean @@ -481,13 +482,13 @@ template ::value>::type* = nullptr> bool& GetImpl(T& val) { // NOLINT - return val.getBoolean(); + return val.GetBoolean(); } template ::value>::type* = nullptr> bool const& GetImpl(T& val) { // NOLINT - return val.getBoolean(); + return val.GetBoolean(); } // Array @@ -495,13 +496,13 @@ template ::value>::type* = nullptr> std::vector& GetImpl(T& val) { // NOLINT - return val.getArray(); + return val.GetArray(); } template ::value>::type* = nullptr> std::vector const& GetImpl(T& val) { // NOLINT - return val.getArray(); + return val.GetArray(); } // Object @@ -509,13 +510,13 @@ template ::value>::type* = nullptr> std::map& GetImpl(T& val) { // NOLINT - return val.getObject(); + return val.GetObject(); } template ::value>::type* = nullptr> std::map const& GetImpl(T& val) { // NOLINT - return val.getObject(); + return val.GetObject(); } } // namespace detail @@ -545,7 +546,7 @@ using Null = JsonNull; // Utils tailored for XGBoost. template -Object toJson(Parameter const& param) { +Object ToJson(Parameter const& param) { Object obj; for (auto const& kv : param.__DICT__()) { obj[kv.first] = kv.second; @@ -554,7 +555,7 @@ Object toJson(Parameter const& param) { } template -void fromJson(Json const& obj, Parameter* param) { +void FromJson(Json const& obj, Parameter* param) { auto const& j_param = get(obj); std::map m; for (auto const& kv : j_param) { diff --git a/include/xgboost/json_io.h b/include/xgboost/json_io.h index ad3dabdb9..a58afe172 100644 --- a/include/xgboost/json_io.h +++ b/include/xgboost/json_io.h @@ -38,10 +38,11 @@ class JsonReader { std::numeric_limits::max_digits10 + 1; struct SourceLocation { - size_t pos_; // current position in raw_str_ + private: + size_t pos_ { 0 }; // current position in raw_str_ public: - SourceLocation() : pos_(0) {} + SourceLocation() = default; size_t Pos() const { return pos_; } SourceLocation& Forward() { diff --git a/include/xgboost/learner.h b/include/xgboost/learner.h index 5985a5c12..a608bc1b8 100644 --- a/include/xgboost/learner.h +++ b/include/xgboost/learner.h @@ -234,13 +234,13 @@ struct LearnerModelParamLegacy; */ struct LearnerModelParam { /* \brief global bias */ - bst_float base_score; + bst_float base_score { 0.5f }; /* \brief number of features */ - uint32_t num_feature; + uint32_t num_feature { 0 }; /* \brief number of classes, if it is multi-class classification */ - uint32_t num_output_group; + uint32_t num_output_group { 0 }; - LearnerModelParam() : base_score {0.5}, num_feature{0}, num_output_group{0} {} + LearnerModelParam() = default; // As the old `LearnerModelParamLegacy` is still used by binary IO, we keep // this one as an immutable copy. LearnerModelParam(LearnerModelParamLegacy const& user_param, float base_margin); diff --git a/include/xgboost/linear_updater.h b/include/xgboost/linear_updater.h index 30803b96b..39a0c324a 100644 --- a/include/xgboost/linear_updater.h +++ b/include/xgboost/linear_updater.h @@ -33,7 +33,7 @@ class LinearUpdater : public Configurable { public: /*! \brief virtual destructor */ - virtual ~LinearUpdater() = default; + ~LinearUpdater() override = default; /*! * \brief Initialize the updater with given arguments. * \param args arguments to the objective function. diff --git a/include/xgboost/metric.h b/include/xgboost/metric.h index 8ecc73c69..8e9d2afe6 100644 --- a/include/xgboost/metric.h +++ b/include/xgboost/metric.h @@ -41,14 +41,14 @@ class Metric : public Configurable { * override this function to maintain internal configuration * \param in JSON object containing the configuration */ - virtual void LoadConfig(Json const& in) {} + void LoadConfig(Json const& in) override {} /*! * \brief Save configuration to JSON object * By default, metric has no internal configuration; * override this function to maintain internal configuration * \param out pointer to output JSON object */ - virtual void SaveConfig(Json* out) const {} + void SaveConfig(Json* out) const override {} /*! * \brief evaluate a specific metric @@ -64,7 +64,7 @@ class Metric : public Configurable { /*! \return name of metric */ virtual const char* Name() const = 0; /*! \brief virtual destructor */ - virtual ~Metric() = default; + ~Metric() override = default; /*! * \brief create a metric according to name. * \param name name of the metric. diff --git a/include/xgboost/objective.h b/include/xgboost/objective.h index e55a63542..1c0942ed0 100644 --- a/include/xgboost/objective.h +++ b/include/xgboost/objective.h @@ -28,7 +28,7 @@ class ObjFunction : public Configurable { public: /*! \brief virtual destructor */ - virtual ~ObjFunction() = default; + ~ObjFunction() override = default; /*! * \brief Configure the objective with the specified parameters. * \param args arguments to the objective function. diff --git a/include/xgboost/predictor.h b/include/xgboost/predictor.h index 76883c30a..0e3f7568c 100644 --- a/include/xgboost/predictor.h +++ b/include/xgboost/predictor.h @@ -36,11 +36,11 @@ struct PredictionCacheEntry { // A storage for caching prediction values HostDeviceVector predictions; // The version of current cache, corresponding number of layers of trees - uint32_t version; + uint32_t version { 0 }; // A weak pointer for checking whether the DMatrix object has expired. std::weak_ptr< DMatrix > ref; - PredictionCacheEntry() : version { 0 } {} + PredictionCacheEntry() = default; /* \brief Update the cache entry by number of versions. * * \param v Added versions. diff --git a/include/xgboost/span.h b/include/xgboost/span.h index 1750ac2c4..4850d7543 100644 --- a/include/xgboost/span.h +++ b/include/xgboost/span.h @@ -105,8 +105,9 @@ namespace detail { * represent ptrdiff_t, which is just int64_t. So we make it determinstic * here. */ -using ptrdiff_t = typename std::conditional::value, - std::ptrdiff_t, std::int64_t>::type; +using ptrdiff_t = typename std::conditional< // NOLINT + std::is_same::value, + std::ptrdiff_t, std::int64_t>::type; } // namespace detail #if defined(_MSC_VER) && _MSC_VER < 1910 @@ -136,7 +137,7 @@ class SpanIterator { IsConst, const ElementType, ElementType>::type&; using pointer = typename std::add_pointer::type; // NOLINT - XGBOOST_DEVICE constexpr SpanIterator() : span_{nullptr}, index_{0} {} + constexpr SpanIterator() = default; XGBOOST_DEVICE constexpr SpanIterator( const SpanType* _span, @@ -243,8 +244,8 @@ class SpanIterator { } protected: - const SpanType *span_; - typename SpanType::index_type index_; + const SpanType *span_ { nullptr }; + typename SpanType::index_type index_ { 0 }; }; @@ -409,8 +410,7 @@ class Span { using const_reverse_iterator = const detail::SpanIterator, true>; // NOLINT // constructors - - XGBOOST_DEVICE constexpr Span() __span_noexcept : size_(0), data_(nullptr) {} + constexpr Span() __span_noexcept = default; XGBOOST_DEVICE Span(pointer _ptr, index_type _count) : size_(_count), data_(_ptr) { @@ -503,11 +503,11 @@ class Span { // element access - XGBOOST_DEVICE reference front() const { + XGBOOST_DEVICE reference front() const { // NOLINT return (*this)[0]; } - XGBOOST_DEVICE reference back() const { + XGBOOST_DEVICE reference back() const { // NOLINT return (*this)[size() - 1]; } @@ -587,8 +587,8 @@ class Span { } private: - index_type size_; - pointer data_; + index_type size_ { 0 }; + pointer data_ { nullptr }; }; template diff --git a/include/xgboost/tree_updater.h b/include/xgboost/tree_updater.h index 69f972279..a091c81b0 100644 --- a/include/xgboost/tree_updater.h +++ b/include/xgboost/tree_updater.h @@ -34,7 +34,7 @@ class TreeUpdater : public Configurable { public: /*! \brief virtual destructor */ - virtual ~TreeUpdater() = default; + ~TreeUpdater() override = default; /*! * \brief Initialize the updater with given arguments. * \param args arguments to the objective function. diff --git a/plugin/example/custom_obj.cc b/plugin/example/custom_obj.cc index 2d4510f1c..57dc2f499 100644 --- a/plugin/example/custom_obj.cc +++ b/plugin/example/custom_obj.cc @@ -73,11 +73,11 @@ class MyLogistic : public ObjFunction { void SaveConfig(Json* p_out) const override { auto& out = *p_out; out["name"] = String("my_logistic"); - out["my_logistic_param"] = toJson(param_); + out["my_logistic_param"] = ToJson(param_); } void LoadConfig(Json const& in) override { - fromJson(in["my_logistic_param"], ¶m_); + FromJson(in["my_logistic_param"], ¶m_); } private: diff --git a/src/common/bitfield.h b/src/common/bitfield.h index 54e00aba6..4353a5269 100644 --- a/src/common/bitfield.h +++ b/src/common/bitfield.h @@ -56,8 +56,8 @@ __forceinline__ __device__ BitFieldAtomicType AtomicAnd(BitFieldAtomicType* addr */ template struct BitFieldContainer { - using value_type = VT; - using pointer = value_type*; + using value_type = VT; // NOLINT + using pointer = value_type*; // NOLINT static value_type constexpr kValueSize = sizeof(value_type) * 8; static value_type constexpr kOne = 1; // force correct type. @@ -67,6 +67,7 @@ struct BitFieldContainer { value_type bit_pos {0}; }; + private: common::Span bits_; static_assert(!std::is_signed::value, "Must use unsiged type as underlying storage."); @@ -82,9 +83,12 @@ struct BitFieldContainer { public: BitFieldContainer() = default; - XGBOOST_DEVICE BitFieldContainer(common::Span bits) : bits_{bits} {} + XGBOOST_DEVICE explicit BitFieldContainer(common::Span bits) : bits_{bits} {} XGBOOST_DEVICE BitFieldContainer(BitFieldContainer const& other) : bits_{other.bits_} {} + common::Span Bits() { return bits_; } + common::Span Bits() const { return bits_; } + /*\brief Compute the size of needed memory allocation. The returned value is in terms * of number of elements with `BitFieldContainer::value_type'. */ @@ -190,7 +194,7 @@ template struct LBitsPolicy : public BitFieldContainer> { using Container = BitFieldContainer>; using Pos = typename Container::Pos; - using value_type = typename Container::value_type; + using value_type = typename Container::value_type; // NOLINT XGBOOST_DEVICE static Pos Shift(Pos pos) { pos.bit_pos = Container::kValueSize - pos.bit_pos - Container::kOne; @@ -204,7 +208,7 @@ template struct RBitsPolicy : public BitFieldContainer> { using Container = BitFieldContainer>; using Pos = typename Container::Pos; - using value_type = typename Container::value_type; + using value_type = typename Container::value_type; // NOLINT XGBOOST_DEVICE static Pos Shift(Pos pos) { return pos; diff --git a/src/common/column_matrix.h b/src/common/column_matrix.h index 6ddeeac03..e179b0505 100644 --- a/src/common/column_matrix.h +++ b/src/common/column_matrix.h @@ -141,7 +141,7 @@ class ColumnMatrix { feature_offsets_[fid] = accum_index_; } - SetTypeSize(gmat.max_num_bins_); + SetTypeSize(gmat.max_num_bins); index_.resize(feature_offsets_[nfeature] * bins_type_size_, 0); if (!all_dense) { @@ -161,24 +161,24 @@ class ColumnMatrix { // pre-fill index_ for dense columns if (all_dense) { - BinTypeSize gmat_bin_size = gmat.index.getBinTypeSize(); - if (gmat_bin_size == UINT8_BINS_TYPE_SIZE) { + BinTypeSize gmat_bin_size = gmat.index.GetBinTypeSize(); + if (gmat_bin_size == kUint8BinsTypeSize) { SetIndexAllDense(gmat.index.data(), gmat, nrow, nfeature, noMissingValues); - } else if (gmat_bin_size == UINT16_BINS_TYPE_SIZE) { + } else if (gmat_bin_size == kUint16BinsTypeSize) { SetIndexAllDense(gmat.index.data(), gmat, nrow, nfeature, noMissingValues); } else { - CHECK_EQ(gmat_bin_size, UINT32_BINS_TYPE_SIZE); + CHECK_EQ(gmat_bin_size, kUint32BinsTypeSize); SetIndexAllDense(gmat.index.data(), gmat, nrow, nfeature, noMissingValues); } - /* For sparse DMatrix gmat.index.getBinTypeSize() returns always UINT32_BINS_TYPE_SIZE + /* For sparse DMatrix gmat.index.getBinTypeSize() returns always kUint32BinsTypeSize but for ColumnMatrix we still have a chance to reduce the memory consumption */ } else { - if (bins_type_size_ == UINT8_BINS_TYPE_SIZE) { + if (bins_type_size_ == kUint8BinsTypeSize) { SetIndex(gmat.index.data(), gmat, nrow, nfeature); - } else if (bins_type_size_ == UINT16_BINS_TYPE_SIZE) { + } else if (bins_type_size_ == kUint16BinsTypeSize) { SetIndex(gmat.index.data(), gmat, nrow, nfeature); } else { - CHECK_EQ(bins_type_size_, UINT32_BINS_TYPE_SIZE); + CHECK_EQ(bins_type_size_, kUint32BinsTypeSize); SetIndex(gmat.index.data(), gmat, nrow, nfeature); } } @@ -187,11 +187,11 @@ class ColumnMatrix { /* Set the number of bytes based on numeric limit of maximum number of bins provided by user */ void SetTypeSize(size_t max_num_bins) { if ( (max_num_bins - 1) <= static_cast(std::numeric_limits::max()) ) { - bins_type_size_ = UINT8_BINS_TYPE_SIZE; + bins_type_size_ = kUint8BinsTypeSize; } else if ((max_num_bins - 1) <= static_cast(std::numeric_limits::max())) { - bins_type_size_ = UINT16_BINS_TYPE_SIZE; + bins_type_size_ = kUint16BinsTypeSize; } else { - bins_type_size_ = UINT32_BINS_TYPE_SIZE; + bins_type_size_ = kUint32BinsTypeSize; } } @@ -227,7 +227,7 @@ class ColumnMatrix { /* missing values make sense only for column with type kDenseColumn, and if no missing values were observed it could be handled much faster. */ if (noMissingValues) { - const int32_t nthread = omp_get_max_threads(); + const int32_t nthread = omp_get_max_threads(); // NOLINT #pragma omp parallel for num_threads(nthread) for (omp_ulong rid = 0; rid < nrow; ++rid) { const size_t ibegin = rid*nfeature; @@ -241,7 +241,7 @@ class ColumnMatrix { } else { /* to handle rows in all batches, sum of all batch sizes equal to gmat.row_ptr.size() - 1 */ size_t rbegin = 0; - for (const auto &batch : gmat.p_fmat_->GetBatches()) { + for (const auto &batch : gmat.p_fmat->GetBatches()) { const xgboost::Entry* data_ptr = batch.data.HostVector().data(); const std::vector& offset_vec = batch.offset.HostVector(); const size_t batch_size = batch.Size(); @@ -276,7 +276,7 @@ class ColumnMatrix { T* local_index = reinterpret_cast(&index_[0]); size_t rbegin = 0; - for (const auto &batch : gmat.p_fmat_->GetBatches()) { + for (const auto &batch : gmat.p_fmat->GetBatches()) { const xgboost::Entry* data_ptr = batch.data.HostVector().data(); const std::vector& offset_vec = batch.offset.HostVector(); const size_t batch_size = batch.Size(); diff --git a/src/common/common.h b/src/common/common.h index 4be2c48aa..adc24519e 100644 --- a/src/common/common.h +++ b/src/common/common.h @@ -118,7 +118,7 @@ class Range { XGBOOST_DEVICE explicit Iterator(DifferenceType start, DifferenceType step) : i_{start}, step_{step} {} - public: + private: int64_t i_; DifferenceType step_ = 1; }; diff --git a/src/common/compressed_iterator.h b/src/common/compressed_iterator.h index eea7eb35d..9f60722fb 100644 --- a/src/common/compressed_iterator.h +++ b/src/common/compressed_iterator.h @@ -112,7 +112,7 @@ class CompressedBufferWriter { size_t ibyte_start = ibit_start / 8, ibyte_end = ibit_end / 8; symbol <<= 7 - ibit_end % 8; - for (ptrdiff_t ibyte = ibyte_end; ibyte >= (ptrdiff_t)ibyte_start; --ibyte) { + for (ptrdiff_t ibyte = ibyte_end; ibyte >= static_cast(ibyte_start); --ibyte) { dh::AtomicOrByte(reinterpret_cast(buffer + detail::kPadding), ibyte, symbol & 0xff); symbol >>= 8; @@ -182,14 +182,14 @@ class CompressedIterator { typedef value_type reference; // NOLINT private: - const CompressedByteT *buffer_; - size_t symbol_bits_; - size_t offset_; + const CompressedByteT *buffer_ {nullptr}; + size_t symbol_bits_ {0}; + size_t offset_ {0}; public: - CompressedIterator() : buffer_(nullptr), symbol_bits_(0), offset_(0) {} + CompressedIterator() = default; CompressedIterator(const CompressedByteT *buffer, size_t num_symbols) - : buffer_(buffer), offset_(0) { + : buffer_(buffer) { symbol_bits_ = detail::SymbolBits(num_symbols); } diff --git a/src/common/config.h b/src/common/config.h index 2efe35a4f..f10d86510 100644 --- a/src/common/config.h +++ b/src/common/config.h @@ -29,8 +29,8 @@ class ConfigParser { * \brief Constructor for INI-style configuration parser * \param path path to configuration file */ - explicit ConfigParser(const std::string& path) - : path_(path), + explicit ConfigParser(const std::string path) + : path_(std::move(path)), line_comment_regex_("^#"), key_regex_(R"rx(^([^#"'=\r\n\t ]+)[\t ]*=)rx"), key_regex_escaped_(R"rx(^(["'])([^"'=\r\n]+)\1[\t ]*=)rx"), @@ -58,12 +58,12 @@ class ConfigParser { std::string NormalizeConfigEOL(std::string const& config_str) { std::string result; std::stringstream ss(config_str); - for (size_t i = 0; i < config_str.size(); ++i) { - if (config_str[i] == '\r') { + for (auto c : config_str) { + if (c == '\r') { result.push_back('\n'); continue; } - result.push_back(config_str[i]); + result.push_back(c); } return result; } diff --git a/src/common/device_helpers.cu b/src/common/device_helpers.cu index c30c55d90..c005c5a91 100644 --- a/src/common/device_helpers.cu +++ b/src/common/device_helpers.cu @@ -37,7 +37,7 @@ void AllReducer::Init(int _device_ordinal) { #ifdef XGBOOST_USE_NCCL LOG(DEBUG) << "Running nccl init on: " << __CUDACC_VER_MAJOR__ << "." << __CUDACC_VER_MINOR__; - device_ordinal = _device_ordinal; + device_ordinal_ = _device_ordinal; int32_t const rank = rabit::GetRank(); #if __CUDACC_VER_MAJOR__ > 9 @@ -46,7 +46,7 @@ void AllReducer::Init(int _device_ordinal) { std::vector uuids(world * kUuidLength, 0); auto s_uuid = xgboost::common::Span{uuids.data(), uuids.size()}; auto s_this_uuid = s_uuid.subspan(rank * kUuidLength, kUuidLength); - GetCudaUUID(world, rank, device_ordinal, s_this_uuid); + GetCudaUUID(world, rank, device_ordinal_, s_this_uuid); // No allgather yet. rabit::Allreduce(uuids.data(), uuids.size()); @@ -66,10 +66,10 @@ void AllReducer::Init(int _device_ordinal) { << "device is not supported"; #endif // __CUDACC_VER_MAJOR__ > 9 - id = GetUniqueId(); - dh::safe_cuda(cudaSetDevice(device_ordinal)); - dh::safe_nccl(ncclCommInitRank(&comm, rabit::GetWorldSize(), id, rank)); - safe_cuda(cudaStreamCreate(&stream)); + id_ = GetUniqueId(); + dh::safe_cuda(cudaSetDevice(device_ordinal_)); + dh::safe_nccl(ncclCommInitRank(&comm_, rabit::GetWorldSize(), id_, rank)); + safe_cuda(cudaStreamCreate(&stream_)); initialised_ = true; #else if (rabit::IsDistributed()) { @@ -81,8 +81,8 @@ void AllReducer::Init(int _device_ordinal) { AllReducer::~AllReducer() { #ifdef XGBOOST_USE_NCCL if (initialised_) { - dh::safe_cuda(cudaStreamDestroy(stream)); - ncclCommDestroy(comm); + dh::safe_cuda(cudaStreamDestroy(stream_)); + ncclCommDestroy(comm_); } if (xgboost::ConsoleLogger::ShouldLog(xgboost::ConsoleLogger::LV::kDebug)) { LOG(CONSOLE) << "======== NCCL Statistics========"; diff --git a/src/common/device_helpers.cuh b/src/common/device_helpers.cuh index ee67e7793..6d51c2097 100644 --- a/src/common/device_helpers.cuh +++ b/src/common/device_helpers.cuh @@ -35,10 +35,10 @@ #include "../common/io.h" #endif -#if !defined(__CUDA_ARCH__) || __CUDA_ARCH__ >= 600 +#if !defined(__CUDA_ARCH__) || __CUDA_ARCH__ >= 600 || defined(__clang__) #else // In device code and CUDA < 600 -XGBOOST_DEVICE __forceinline__ double atomicAdd(double* address, double val) { +__device__ __forceinline__ double atomicAdd(double* address, double val) { // NOLINT unsigned long long int* address_as_ull = (unsigned long long int*)address; // NOLINT unsigned long long int old = *address_as_ull, assumed; // NOLINT @@ -141,7 +141,8 @@ inline void CheckComputeCapability() { } DEV_INLINE void AtomicOrByte(unsigned int* __restrict__ buffer, size_t ibyte, unsigned char b) { - atomicOr(&buffer[ibyte / sizeof(unsigned int)], (unsigned int)b << (ibyte % (sizeof(unsigned int)) * 8)); + atomicOr(&buffer[ibyte / sizeof(unsigned int)], + static_cast(b) << (ibyte % (sizeof(unsigned int)) * 8)); } namespace internal { @@ -174,7 +175,7 @@ CountNumItemsImpl(bool left, const T * __restrict__ items, uint32_t n, T v, return left ? items_begin - items : items + n - items_begin; } -} +} // namespace internal /*! * \brief Find the strict upper bound for an element in a sorted array @@ -291,9 +292,9 @@ class LaunchKernel { dim3 blocks_; public: - LaunchKernel(uint32_t _grids, uint32_t _blk, size_t _shmem=0, cudaStream_t _s=0) : + LaunchKernel(uint32_t _grids, uint32_t _blk, size_t _shmem=0, cudaStream_t _s=nullptr) : grids_{_grids, 1, 1}, blocks_{_blk, 1, 1}, shmem_size_{_shmem}, stream_{_s} {} - LaunchKernel(dim3 _grids, dim3 _blk, size_t _shmem=0, cudaStream_t _s=0) : + LaunchKernel(dim3 _grids, dim3 _blk, size_t _shmem=0, cudaStream_t _s=nullptr) : grids_{_grids}, blocks_{_blk}, shmem_size_{_shmem}, stream_{_s} {} template @@ -359,16 +360,18 @@ class MemoryLogger { public: void RegisterAllocation(void *ptr, size_t n) { - if (!xgboost::ConsoleLogger::ShouldLog(xgboost::ConsoleLogger::LV::kDebug)) + if (!xgboost::ConsoleLogger::ShouldLog(xgboost::ConsoleLogger::LV::kDebug)) { return; + } std::lock_guard guard(mutex_); int current_device; safe_cuda(cudaGetDevice(¤t_device)); stats_.RegisterAllocation(ptr, n); } void RegisterDeallocation(void *ptr, size_t n) { - if (!xgboost::ConsoleLogger::ShouldLog(xgboost::ConsoleLogger::LV::kDebug)) + if (!xgboost::ConsoleLogger::ShouldLog(xgboost::ConsoleLogger::LV::kDebug)) { return; + } std::lock_guard guard(mutex_); int current_device; safe_cuda(cudaGetDevice(¤t_device)); @@ -384,8 +387,9 @@ public: } void Log() { - if (!xgboost::ConsoleLogger::ShouldLog(xgboost::ConsoleLogger::LV::kDebug)) + if (!xgboost::ConsoleLogger::ShouldLog(xgboost::ConsoleLogger::LV::kDebug)) { return; + } std::lock_guard guard(mutex_); int current_device; safe_cuda(cudaGetDevice(¤t_device)); @@ -396,7 +400,7 @@ public: LOG(CONSOLE) << "Number of allocations: " << stats_.num_allocations; } }; -}; +} // namespace detail inline detail::MemoryLogger &GlobalMemoryLogger() { static detail::MemoryLogger memory_logger; @@ -413,27 +417,27 @@ inline void DebugSyncDevice(std::string file="", int32_t line = -1) { safe_cuda(cudaGetLastError()); } -namespace detail{ +namespace detail { /** * \brief Default memory allocator, uses cudaMalloc/Free and logs allocations if verbose. */ template struct XGBDefaultDeviceAllocatorImpl : thrust::device_malloc_allocator { - using super_t = thrust::device_malloc_allocator; - using pointer = thrust::device_ptr; + using SuperT = thrust::device_malloc_allocator; + using pointer = thrust::device_ptr; // NOLINT template - struct rebind + struct rebind // NOLINT { - typedef XGBDefaultDeviceAllocatorImpl other; + using other = XGBDefaultDeviceAllocatorImpl; // NOLINT }; - pointer allocate(size_t n) { - pointer ptr = super_t::allocate(n); + pointer allocate(size_t n) { // NOLINT + pointer ptr = SuperT::allocate(n); GlobalMemoryLogger().RegisterAllocation(ptr.get(), n * sizeof(T)); return ptr; } - void deallocate(pointer ptr, size_t n) { + void deallocate(pointer ptr, size_t n) { // NOLINT GlobalMemoryLogger().RegisterDeallocation(ptr.get(), n * sizeof(T)); - return super_t::deallocate(ptr, n); + return SuperT::deallocate(ptr, n); } }; @@ -442,11 +446,11 @@ struct XGBDefaultDeviceAllocatorImpl : thrust::device_malloc_allocator { */ template struct XGBCachingDeviceAllocatorImpl : thrust::device_malloc_allocator { - using pointer = thrust::device_ptr; + using pointer = thrust::device_ptr; // NOLINT template - struct rebind + struct rebind // NOLINT { - typedef XGBCachingDeviceAllocatorImpl other; + using other = XGBCachingDeviceAllocatorImpl; // NOLINT }; cub::CachingDeviceAllocator& GetGlobalCachingAllocator () { @@ -455,7 +459,7 @@ struct XGBCachingDeviceAllocatorImpl : thrust::device_malloc_allocator { static cub::CachingDeviceAllocator *allocator = new cub::CachingDeviceAllocator(2, 9, 29); return *allocator; } - pointer allocate(size_t n) { + pointer allocate(size_t n) { // NOLINT T *ptr; GetGlobalCachingAllocator().DeviceAllocate(reinterpret_cast(&ptr), n * sizeof(T)); @@ -463,17 +467,17 @@ struct XGBCachingDeviceAllocatorImpl : thrust::device_malloc_allocator { GlobalMemoryLogger().RegisterAllocation(thrust_ptr.get(), n * sizeof(T)); return thrust_ptr; } - void deallocate(pointer ptr, size_t n) { + void deallocate(pointer ptr, size_t n) { // NOLINT GlobalMemoryLogger().RegisterDeallocation(ptr.get(), n * sizeof(T)); GetGlobalCachingAllocator().DeviceFree(ptr.get()); } __host__ __device__ - void construct(T *) + void construct(T *) // NOLINT { // no-op } }; -}; +} // namespace detail // Declare xgboost allocators // Replacement of allocator with custom backend should occur here @@ -486,9 +490,9 @@ template using XGBCachingDeviceAllocator = detail::XGBCachingDeviceAllocatorImpl; /** \brief Specialisation of thrust device vector using custom allocator. */ template -using device_vector = thrust::device_vector>; +using device_vector = thrust::device_vector>; // NOLINT template -using caching_device_vector = thrust::device_vector>; +using caching_device_vector = thrust::device_vector>; // NOLINT /** * \brief A double buffer, useful for algorithms like sort. @@ -517,7 +521,7 @@ class DoubleBuffer { return xgboost::common::Span{buff.Current(), Size()}; } - T *other() { return buff.Alternate(); } + T *Other() { return buff.Alternate(); } }; /** @@ -688,7 +692,9 @@ class BulkAllocator { template void Allocate(int device_idx, Args... args) { - if (device_idx_ == -1) device_idx_ = device_idx; + if (device_idx_ == -1) { + device_idx_ = device_idx; + } else CHECK(device_idx_ == device_idx); size_t size = GetSizeBytes(args...); @@ -728,13 +734,13 @@ struct PinnedMemory { // Keep track of cub library device allocation struct CubMemory { - void *d_temp_storage; - size_t temp_storage_bytes; + void *d_temp_storage { nullptr }; + size_t temp_storage_bytes { 0 }; // Thrust using value_type = char; // NOLINT - CubMemory() : d_temp_storage(nullptr), temp_storage_bytes(0) {} + CubMemory() = default; ~CubMemory() { Free(); } @@ -818,7 +824,7 @@ __global__ void LbsKernel(CoordinateT *d_coordinates, cub::CountingInputIterator tile_element_indices(tile_start_coord.y); CoordinateT thread_start_coord; - typedef typename std::iterator_traits::value_type SegmentT; + using SegmentT = typename std::iterator_traits::value_type; __shared__ struct { SegmentT tile_segment_end_offsets[TILE_SIZE + 1]; SegmentT output_segment[TILE_SIZE]; @@ -862,7 +868,7 @@ template void SparseTransformLbs(int device_idx, dh::CubMemory *temp_memory, OffsetT count, SegmentIterT segments, OffsetT num_segments, FunctionT f) { - typedef typename cub::CubVector::Type CoordinateT; + using CoordinateT = typename cub::CubVector::Type; dh::safe_cuda(cudaSetDevice(device_idx)); const int BLOCK_THREADS = 256; const int ITEMS_PER_THREAD = 1; @@ -961,13 +967,13 @@ void SegmentedSort(dh::CubMemory *tmp_mem, dh::DoubleBuffer *keys, * @param nVals number of elements in the input array */ template -void SumReduction(dh::CubMemory &tmp_mem, xgboost::common::Span in, xgboost::common::Span out, +void SumReduction(dh::CubMemory* tmp_mem, xgboost::common::Span in, xgboost::common::Span out, int nVals) { size_t tmpSize; dh::safe_cuda( cub::DeviceReduce::Sum(NULL, tmpSize, in.data(), out.data(), nVals)); - tmp_mem.LazyAllocate(tmpSize); - dh::safe_cuda(cub::DeviceReduce::Sum(tmp_mem.d_temp_storage, tmpSize, + tmp_mem->LazyAllocate(tmpSize); + dh::safe_cuda(cub::DeviceReduce::Sum(tmp_mem->d_temp_storage, tmpSize, in.data(), out.data(), nVals)); } @@ -980,20 +986,20 @@ void SumReduction(dh::CubMemory &tmp_mem, xgboost::common::Span in, xgboost:: */ template typename std::iterator_traits::value_type SumReduction( - dh::CubMemory &tmp_mem, T in, int nVals) { + dh::CubMemory* tmp_mem, T in, int nVals) { using ValueT = typename std::iterator_traits::value_type; size_t tmpSize {0}; ValueT *dummy_out = nullptr; dh::safe_cuda(cub::DeviceReduce::Sum(nullptr, tmpSize, in, dummy_out, nVals)); // Allocate small extra memory for the return value - tmp_mem.LazyAllocate(tmpSize + sizeof(ValueT)); - auto ptr = reinterpret_cast(tmp_mem.d_temp_storage) + 1; + tmp_mem->LazyAllocate(tmpSize + sizeof(ValueT)); + auto ptr = reinterpret_cast(tmp_mem->d_temp_storage) + 1; dh::safe_cuda(cub::DeviceReduce::Sum( reinterpret_cast(ptr), tmpSize, in, - reinterpret_cast(tmp_mem.d_temp_storage), + reinterpret_cast(tmp_mem->d_temp_storage), nVals)); ValueT sum; - dh::safe_cuda(cudaMemcpy(&sum, tmp_mem.d_temp_storage, sizeof(ValueT), + dh::safe_cuda(cudaMemcpy(&sum, tmp_mem->d_temp_storage, sizeof(ValueT), cudaMemcpyDeviceToHost)); return sum; } @@ -1079,20 +1085,19 @@ class SaveCudaContext { * this is a dummy class that will error if used with more than one GPU. */ class AllReducer { - bool initialised_; - size_t allreduce_bytes_; // Keep statistics of the number of bytes communicated - size_t allreduce_calls_; // Keep statistics of the number of reduce calls - std::vector host_data; // Used for all reduce on host + bool initialised_ {false}; + size_t allreduce_bytes_ {0}; // Keep statistics of the number of bytes communicated + size_t allreduce_calls_ {0}; // Keep statistics of the number of reduce calls + std::vector host_data_; // Used for all reduce on host #ifdef XGBOOST_USE_NCCL - ncclComm_t comm; - cudaStream_t stream; - int device_ordinal; - ncclUniqueId id; + ncclComm_t comm_; + cudaStream_t stream_; + int device_ordinal_; + ncclUniqueId id_; #endif public: - AllReducer() : initialised_(false), allreduce_bytes_(0), - allreduce_calls_(0) {} + AllReducer() = default; /** * \brief Initialise with the desired device ordinal for this communication @@ -1116,8 +1121,8 @@ class AllReducer { void AllReduceSum(const double *sendbuff, double *recvbuff, int count) { #ifdef XGBOOST_USE_NCCL CHECK(initialised_); - dh::safe_cuda(cudaSetDevice(device_ordinal)); - dh::safe_nccl(ncclAllReduce(sendbuff, recvbuff, count, ncclDouble, ncclSum, comm, stream)); + dh::safe_cuda(cudaSetDevice(device_ordinal_)); + dh::safe_nccl(ncclAllReduce(sendbuff, recvbuff, count, ncclDouble, ncclSum, comm_, stream_)); allreduce_bytes_ += count * sizeof(double); allreduce_calls_ += 1; #endif @@ -1135,8 +1140,8 @@ class AllReducer { void AllReduceSum(const float *sendbuff, float *recvbuff, int count) { #ifdef XGBOOST_USE_NCCL CHECK(initialised_); - dh::safe_cuda(cudaSetDevice(device_ordinal)); - dh::safe_nccl(ncclAllReduce(sendbuff, recvbuff, count, ncclFloat, ncclSum, comm, stream)); + dh::safe_cuda(cudaSetDevice(device_ordinal_)); + dh::safe_nccl(ncclAllReduce(sendbuff, recvbuff, count, ncclFloat, ncclSum, comm_, stream_)); allreduce_bytes_ += count * sizeof(float); allreduce_calls_ += 1; #endif @@ -1156,8 +1161,8 @@ class AllReducer { #ifdef XGBOOST_USE_NCCL CHECK(initialised_); - dh::safe_cuda(cudaSetDevice(device_ordinal)); - dh::safe_nccl(ncclAllReduce(sendbuff, recvbuff, count, ncclInt64, ncclSum, comm, stream)); + dh::safe_cuda(cudaSetDevice(device_ordinal_)); + dh::safe_nccl(ncclAllReduce(sendbuff, recvbuff, count, ncclInt64, ncclSum, comm_, stream_)); #endif } @@ -1168,8 +1173,8 @@ class AllReducer { */ void Synchronize() { #ifdef XGBOOST_USE_NCCL - dh::safe_cuda(cudaSetDevice(device_ordinal)); - dh::safe_cuda(cudaStreamSynchronize(stream)); + dh::safe_cuda(cudaSetDevice(device_ordinal_)); + dh::safe_cuda(cudaStreamSynchronize(stream_)); #endif }; @@ -1183,15 +1188,15 @@ class AllReducer { * \return the Unique ID */ ncclUniqueId GetUniqueId() { - static const int RootRank = 0; + static const int kRootRank = 0; ncclUniqueId id; - if (rabit::GetRank() == RootRank) { + if (rabit::GetRank() == kRootRank) { dh::safe_nccl(ncclGetUniqueId(&id)); } rabit::Broadcast( - (void*)&id, - (size_t)sizeof(ncclUniqueId), - (int)RootRank); + static_cast(&id), + sizeof(ncclUniqueId), + static_cast(kRootRank)); return id; } #endif @@ -1202,18 +1207,18 @@ class AllReducer { void HostMaxAllReduce(std::vector *p_data) { #ifdef XGBOOST_USE_NCCL auto &data = *p_data; - // Wait in case some other thread is accessing host_data + // Wait in case some other thread is accessing host_data_ #pragma omp barrier // Reset shared buffer #pragma omp single { - host_data.resize(data.size()); - std::fill(host_data.begin(), host_data.end(), size_t(0)); + host_data_.resize(data.size()); + std::fill(host_data_.begin(), host_data_.end(), size_t(0)); } // Threads update shared array for (auto i = 0ull; i < data.size(); i++) { #pragma omp critical - { host_data[i] = std::max(host_data[i], data[i]); } + { host_data_[i] = std::max(host_data_[i], data[i]); } } // Wait until all threads are finished #pragma omp barrier @@ -1221,15 +1226,15 @@ class AllReducer { // One thread performs all reduce across distributed nodes #pragma omp master { - rabit::Allreduce(host_data.data(), - host_data.size()); + rabit::Allreduce(host_data_.data(), + host_data_.size()); } #pragma omp barrier // Threads can now read back all reduced values for (auto i = 0ull; i < data.size(); i++) { - data[i] = host_data[i]; + data[i] = host_data_[i]; } #endif } @@ -1264,12 +1269,12 @@ thrust::device_ptr tend(xgboost::HostDeviceVector& vector) { // // NOLINT } template -thrust::device_ptr tcbegin(xgboost::HostDeviceVector const& vector) { +thrust::device_ptr tcbegin(xgboost::HostDeviceVector const& vector) { // NOLINT return thrust::device_ptr(vector.ConstDevicePointer()); } template -thrust::device_ptr tcend(xgboost::HostDeviceVector const& vector) { +thrust::device_ptr tcend(xgboost::HostDeviceVector const& vector) { // NOLINT return tcbegin(vector) + vector.Size(); } @@ -1279,17 +1284,17 @@ thrust::device_ptr tbegin(xgboost::common::Span& span) { // NOLINT } template -thrust::device_ptr tend(xgboost::common::Span& span) { // // NOLINT +thrust::device_ptr tend(xgboost::common::Span& span) { // NOLINT return tbegin(span) + span.size(); } template -thrust::device_ptr tcbegin(xgboost::common::Span const& span) { +thrust::device_ptr tcbegin(xgboost::common::Span const& span) { // NOLINT return thrust::device_ptr(span.data()); } template -thrust::device_ptr tcend(xgboost::common::Span const& span) { +thrust::device_ptr tcend(xgboost::common::Span const& span) { // NOLINT return tcbegin(span) + span.size(); } @@ -1465,9 +1470,9 @@ class SegmentSorter { template class LauncherItr { public: - int idx; + int idx { 0 }; FunctionT f; - XGBOOST_DEVICE LauncherItr() : idx(0) {} + XGBOOST_DEVICE LauncherItr() : idx(0) {} // NOLINT XGBOOST_DEVICE LauncherItr(int idx, FunctionT f) : idx(idx), f(f) {} XGBOOST_DEVICE LauncherItr &operator=(int output) { f(idx, output); @@ -1493,7 +1498,7 @@ public: using value_type = void; // NOLINT using pointer = value_type *; // NOLINT using reference = LauncherItr; // NOLINT - using iterator_category = typename thrust::detail::iterator_facade_category< + using iterator_category = typename thrust::detail::iterator_facade_category< // NOLINT thrust::any_system_tag, thrust::random_access_traversal_tag, value_type, reference>::type; // NOLINT private: diff --git a/src/common/hist_util.cc b/src/common/hist_util.cc index e6fbb2188..97c9c9d09 100644 --- a/src/common/hist_util.cc +++ b/src/common/hist_util.cc @@ -1,5 +1,5 @@ /*! - * Copyright 2017-2019 by Contributors + * Copyright 2017-2020 by Contributors * \file hist_util.cc */ #include @@ -11,10 +11,10 @@ #include "xgboost/base.h" #include "../common/common.h" -#include "./hist_util.h" -#include "./random.h" -#include "./column_matrix.h" -#include "./quantile.h" +#include "hist_util.h" +#include "random.h" +#include "column_matrix.h" +#include "quantile.h" #include "./../tree/updater_quantile_hist.h" #if defined(XGBOOST_MM_PREFETCH_PRESENT) @@ -99,16 +99,16 @@ void GHistIndexMatrix::SetIndexDataForSparse(common::Span index_data_s void GHistIndexMatrix::ResizeIndex(const size_t rbegin, const SparsePage& batch, const size_t n_offsets, const size_t n_index, const bool isDense) { - if ((max_num_bins_ - 1 <= static_cast(std::numeric_limits::max())) && isDense) { - index.setBinTypeSize(UINT8_BINS_TYPE_SIZE); - index.resize((sizeof(uint8_t)) * n_index); - } else if ((max_num_bins_ - 1 > static_cast(std::numeric_limits::max()) && - max_num_bins_ - 1 <= static_cast(std::numeric_limits::max())) && isDense) { - index.setBinTypeSize(UINT16_BINS_TYPE_SIZE); - index.resize((sizeof(uint16_t)) * n_index); + if ((max_num_bins - 1 <= static_cast(std::numeric_limits::max())) && isDense) { + index.SetBinTypeSize(kUint8BinsTypeSize); + index.Resize((sizeof(uint8_t)) * n_index); + } else if ((max_num_bins - 1 > static_cast(std::numeric_limits::max()) && + max_num_bins - 1 <= static_cast(std::numeric_limits::max())) && isDense) { + index.SetBinTypeSize(kUint16BinsTypeSize); + index.Resize((sizeof(uint16_t)) * n_index); } else { - index.setBinTypeSize(UINT32_BINS_TYPE_SIZE); - index.resize((sizeof(uint32_t)) * n_index); + index.SetBinTypeSize(kUint32BinsTypeSize); + index.Resize((sizeof(uint32_t)) * n_index); } } @@ -449,15 +449,15 @@ void DenseCuts::Init monitor_.Stop(__func__); } -void GHistIndexMatrix::Init(DMatrix* p_fmat, int max_num_bins) { - cut.Build(p_fmat, max_num_bins); - max_num_bins_ = max_num_bins; +void GHistIndexMatrix::Init(DMatrix* p_fmat, int max_bins) { + cut.Build(p_fmat, max_bins); + max_num_bins = max_bins; const int32_t nthread = omp_get_max_threads(); const uint32_t nbins = cut.Ptrs().back(); hit_count.resize(nbins, 0); hit_count_tloc_.resize(nthread * nbins, 0); - this->p_fmat_ = p_fmat; + this->p_fmat = p_fmat; size_t new_size = 1; for (const auto &batch : p_fmat->GetBatches()) { new_size += batch.Size(); @@ -524,24 +524,24 @@ void GHistIndexMatrix::Init(DMatrix* p_fmat, int max_num_bins) { uint32_t* offsets = nullptr; if (isDense) { - index.resizeOffset(n_offsets); - offsets = index.offset(); + index.ResizeOffset(n_offsets); + offsets = index.Offset(); for (size_t i = 0; i < n_offsets; ++i) { offsets[i] = cut.Ptrs()[i]; } } if (isDense) { - BinTypeSize curent_bin_size = index.getBinTypeSize(); + BinTypeSize curent_bin_size = index.GetBinTypeSize(); common::Span offsets_span = {offsets, n_offsets}; - if (curent_bin_size == UINT8_BINS_TYPE_SIZE) { + if (curent_bin_size == kUint8BinsTypeSize) { common::Span index_data_span = {index.data(), n_index}; SetIndexDataForDense(index_data_span, batch_threads, batch, rbegin, offsets_span, nbins); - } else if (curent_bin_size == UINT16_BINS_TYPE_SIZE) { + } else if (curent_bin_size == kUint16BinsTypeSize) { common::Span index_data_span = {index.data(), n_index}; SetIndexDataForDense(index_data_span, batch_threads, batch, rbegin, offsets_span, nbins); } else { - CHECK_EQ(curent_bin_size, UINT32_BINS_TYPE_SIZE); + CHECK_EQ(curent_bin_size, kUint32BinsTypeSize); common::Span index_data_span = {index.data(), n_index}; SetIndexDataForDense(index_data_span, batch_threads, batch, rbegin, offsets_span, nbins); } @@ -689,16 +689,16 @@ FindGroups(const std::vector& feature_list, } BinTypeSize bins_type_size = colmat.GetTypeSize(); - if (bins_type_size == UINT8_BINS_TYPE_SIZE) { + if (bins_type_size == kUint8BinsTypeSize) { const auto column = colmat.GetColumn(fid); SetGroup(fid, *(column.get()), max_conflict_cnt, search_groups, &group_conflict_cnt, &conflict_marks, &groups, &group_nnz, cur_fid_nnz, nrow); - } else if (bins_type_size == UINT16_BINS_TYPE_SIZE) { + } else if (bins_type_size == kUint16BinsTypeSize) { const auto column = colmat.GetColumn(fid); SetGroup(fid, *(column.get()), max_conflict_cnt, search_groups, &group_conflict_cnt, &conflict_marks, &groups, &group_nnz, cur_fid_nnz, nrow); } else { - CHECK_EQ(bins_type_size, UINT32_BINS_TYPE_SIZE); + CHECK_EQ(bins_type_size, kUint32BinsTypeSize); const auto column = colmat.GetColumn(fid); SetGroup(fid, *(column.get()), max_conflict_cnt, search_groups, &group_conflict_cnt, &conflict_marks, &groups, &group_nnz, cur_fid_nnz, nrow); @@ -909,7 +909,7 @@ void BuildHistDenseKernel(const std::vector& gpair, const size_t* rid = row_indices.begin; const float* pgh = reinterpret_cast(gpair.data()); const BinIdxType* gradient_index = gmat.index.data(); - const uint32_t* offsets = gmat.index.offset(); + const uint32_t* offsets = gmat.index.Offset(); FPType* hist_data = reinterpret_cast(hist.data()); const uint32_t two {2}; // Each element from 'gpair' and 'hist' contains // 2 FP values: gradient and hessian. @@ -1000,16 +1000,16 @@ void BuildHistKernel(const std::vector& gpair, const RowSetCollection::Elem row_indices, const GHistIndexMatrix& gmat, const bool isDense, GHistRow hist) { const bool is_dense = row_indices.Size() && isDense; - switch (gmat.index.getBinTypeSize()) { - case UINT8_BINS_TYPE_SIZE: + switch (gmat.index.GetBinTypeSize()) { + case kUint8BinsTypeSize: BuildHistDispatchKernel(gpair, row_indices, gmat, hist, is_dense); break; - case UINT16_BINS_TYPE_SIZE: + case kUint16BinsTypeSize: BuildHistDispatchKernel(gpair, row_indices, gmat, hist, is_dense); break; - case UINT32_BINS_TYPE_SIZE: + case kUint32BinsTypeSize: BuildHistDispatchKernel(gpair, row_indices, gmat, hist, is_dense); break; diff --git a/src/common/hist_util.h b/src/common/hist_util.h index 827bde431..d94067289 100644 --- a/src/common/hist_util.h +++ b/src/common/hist_util.h @@ -45,9 +45,10 @@ class HistogramCuts { common::Monitor monitor_; public: - HostDeviceVector cut_values_; - HostDeviceVector cut_ptrs_; - HostDeviceVector min_vals_; // storing minimum value in a sketch set. + HostDeviceVector cut_values_; // NOLINT + HostDeviceVector cut_ptrs_; // NOLINT + // storing minimum value in a sketch set. + HostDeviceVector min_vals_; // NOLINT HistogramCuts(); HistogramCuts(HistogramCuts const& that) { @@ -211,14 +212,14 @@ HistogramCuts AdapterDeviceSketch(AdapterT* adapter, int num_bins, enum BinTypeSize { - UINT8_BINS_TYPE_SIZE = 1, - UINT16_BINS_TYPE_SIZE = 2, - UINT32_BINS_TYPE_SIZE = 4 + kUint8BinsTypeSize = 1, + kUint16BinsTypeSize = 2, + kUint32BinsTypeSize = 4 }; struct Index { - Index(): binTypeSize_(UINT8_BINS_TYPE_SIZE), p_(1), offset_ptr_(nullptr) { - setBinTypeSize(binTypeSize_); + Index() { + SetBinTypeSize(binTypeSize_); } Index(const Index& i) = delete; Index& operator=(Index i) = delete; @@ -231,75 +232,75 @@ struct Index { return func_(data_ptr_, i); } } - void setBinTypeSize(BinTypeSize binTypeSize) { + void SetBinTypeSize(BinTypeSize binTypeSize) { binTypeSize_ = binTypeSize; switch (binTypeSize) { - case UINT8_BINS_TYPE_SIZE: - func_ = &getValueFromUint8; + case kUint8BinsTypeSize: + func_ = &GetValueFromUint8; break; - case UINT16_BINS_TYPE_SIZE: - func_ = &getValueFromUint16; + case kUint16BinsTypeSize: + func_ = &GetValueFromUint16; break; - case UINT32_BINS_TYPE_SIZE: - func_ = &getValueFromUint32; + case kUint32BinsTypeSize: + func_ = &GetValueFromUint32; break; default: - CHECK(binTypeSize == UINT8_BINS_TYPE_SIZE || - binTypeSize == UINT16_BINS_TYPE_SIZE || - binTypeSize == UINT32_BINS_TYPE_SIZE); + CHECK(binTypeSize == kUint8BinsTypeSize || + binTypeSize == kUint16BinsTypeSize || + binTypeSize == kUint32BinsTypeSize); } } - BinTypeSize getBinTypeSize() const { + BinTypeSize GetBinTypeSize() const { return binTypeSize_; } template - T* data() const { + T* data() const { // NOLINT return static_cast(data_ptr_); } - uint32_t* offset() const { + uint32_t* Offset() const { return offset_ptr_; } - size_t offsetSize() const { + size_t OffsetSize() const { return offset_.size(); } - size_t size() const { + size_t Size() const { return data_.size() / (binTypeSize_); } - void resize(const size_t nBytesData) { + void Resize(const size_t nBytesData) { data_.resize(nBytesData); data_ptr_ = reinterpret_cast(data_.data()); } - void resizeOffset(const size_t nDisps) { + void ResizeOffset(const size_t nDisps) { offset_.resize(nDisps); offset_ptr_ = offset_.data(); p_ = nDisps; } - std::vector::const_iterator begin() const { + std::vector::const_iterator begin() const { // NOLINT return data_.begin(); } - std::vector::const_iterator end() const { + std::vector::const_iterator end() const { // NOLINT return data_.end(); } private: - static uint32_t getValueFromUint8(void *t, size_t i) { + static uint32_t GetValueFromUint8(void *t, size_t i) { return reinterpret_cast(t)[i]; } - static uint32_t getValueFromUint16(void* t, size_t i) { + static uint32_t GetValueFromUint16(void* t, size_t i) { return reinterpret_cast(t)[i]; } - static uint32_t getValueFromUint32(void* t, size_t i) { + static uint32_t GetValueFromUint32(void* t, size_t i) { return reinterpret_cast(t)[i]; } - typedef uint32_t (*Func)(void*, size_t); + using Func = uint32_t (*)(void*, size_t); std::vector data_; std::vector offset_; // size of this field is equal to number of features void* data_ptr_; - BinTypeSize binTypeSize_; - size_t p_; - uint32_t* offset_ptr_; + BinTypeSize binTypeSize_ {kUint8BinsTypeSize}; + size_t p_ {1}; + uint32_t* offset_ptr_ {nullptr}; Func func_; }; @@ -319,8 +320,8 @@ struct GHistIndexMatrix { std::vector hit_count; /*! \brief The corresponding cuts */ HistogramCuts cut; - DMatrix* p_fmat_; - size_t max_num_bins_; + DMatrix* p_fmat; + size_t max_num_bins; // Create a global histogram matrix, given cut void Init(DMatrix* p_fmat, int max_num_bins); @@ -668,7 +669,7 @@ class ParallelGHistBuilder { */ class GHistBuilder { public: - GHistBuilder() : nthread_{0}, nbins_{0} {} + GHistBuilder() = default; GHistBuilder(size_t nthread, uint32_t nbins) : nthread_{nthread}, nbins_{nbins} {} // construct a histogram via histogram aggregation @@ -691,9 +692,9 @@ class GHistBuilder { private: /*! \brief number of threads for parallel computation */ - size_t nthread_; + size_t nthread_ { 0 }; /*! \brief number of all bins over all features */ - uint32_t nbins_; + uint32_t nbins_ { 0 }; }; diff --git a/src/common/json.cc b/src/common/json.cc index da0d5c012..ca46623d4 100644 --- a/src/common/json.cc +++ b/src/common/json.cc @@ -20,7 +20,7 @@ void JsonWriter::Save(Json json) { void JsonWriter::Visit(JsonArray const* arr) { this->Write("["); - auto const& vec = arr->getArray(); + auto const& vec = arr->GetArray(); size_t size = vec.size(); for (size_t i = 0; i < size; ++i) { auto const& value = vec[i]; @@ -36,9 +36,9 @@ void JsonWriter::Visit(JsonObject const* obj) { this->NewLine(); size_t i = 0; - size_t size = obj->getObject().size(); + size_t size = obj->GetObject().size(); - for (auto& value : obj->getObject()) { + for (auto& value : obj->GetObject()) { this->Write("\"" + value.first + "\":"); this->Save(value.second); @@ -54,14 +54,14 @@ void JsonWriter::Visit(JsonObject const* obj) { } void JsonWriter::Visit(JsonNumber const* num) { - convertor_ << num->getNumber(); + convertor_ << num->GetNumber(); auto const& str = convertor_.str(); this->Write(StringView{str.c_str(), str.size()}); convertor_.str(""); } void JsonWriter::Visit(JsonInteger const* num) { - convertor_ << num->getInteger(); + convertor_ << num->GetInteger(); auto const& str = convertor_.str(); this->Write(StringView{str.c_str(), str.size()}); convertor_.str(""); @@ -74,7 +74,7 @@ void JsonWriter::Visit(JsonNull const* null) { void JsonWriter::Visit(JsonString const* str) { std::string buffer; buffer += '"'; - auto const& string = str->getString(); + auto const& string = str->GetString(); for (size_t i = 0; i < string.length(); i++) { const char ch = string[i]; if (ch == '\\') { @@ -109,7 +109,7 @@ void JsonWriter::Visit(JsonString const* str) { } void JsonWriter::Visit(JsonBoolean const* boolean) { - bool val = boolean->getBoolean(); + bool val = boolean->GetBoolean(); if (val) { this->Write(u8"true"); } else { @@ -120,13 +120,13 @@ void JsonWriter::Visit(JsonBoolean const* boolean) { // Value std::string Value::TypeStr() const { switch (kind_) { - case ValueKind::String: return "String"; break; - case ValueKind::Number: return "Number"; break; - case ValueKind::Object: return "Object"; break; - case ValueKind::Array: return "Array"; break; - case ValueKind::Boolean: return "Boolean"; break; - case ValueKind::Null: return "Null"; break; - case ValueKind::Integer: return "Integer"; break; + case ValueKind::kString: return "String"; break; + case ValueKind::kNumber: return "Number"; break; + case ValueKind::kObject: return "Object"; break; + case ValueKind::kArray: return "Array"; break; + case ValueKind::kBoolean: return "Boolean"; break; + case ValueKind::kNull: return "Null"; break; + case ValueKind::kInteger: return "Integer"; break; } return ""; } @@ -140,10 +140,10 @@ Json& DummyJsonObject() { // Json Object JsonObject::JsonObject(JsonObject && that) : - Value(ValueKind::Object), object_{std::move(that.object_)} {} + Value(ValueKind::kObject), object_{std::move(that.object_)} {} JsonObject::JsonObject(std::map&& object) - : Value(ValueKind::Object), object_{std::move(object)} {} + : Value(ValueKind::kObject), object_{std::move(object)} {} Json& JsonObject::operator[](std::string const & key) { return object_[key]; @@ -157,12 +157,12 @@ Json& JsonObject::operator[](int ind) { bool JsonObject::operator==(Value const& rhs) const { if (!IsA(&rhs)) { return false; } - return object_ == Cast(&rhs)->getObject(); + return object_ == Cast(&rhs)->GetObject(); } Value& JsonObject::operator=(Value const &rhs) { JsonObject const* casted = Cast(&rhs); - object_ = casted->getObject(); + object_ = casted->GetObject(); return *this; } @@ -186,12 +186,12 @@ Json& JsonString::operator[](int ind) { bool JsonString::operator==(Value const& rhs) const { if (!IsA(&rhs)) { return false; } - return Cast(&rhs)->getString() == str_; + return Cast(&rhs)->GetString() == str_; } Value & JsonString::operator=(Value const &rhs) { JsonString const* casted = Cast(&rhs); - str_ = casted->getString(); + str_ = casted->GetString(); return *this; } @@ -202,7 +202,7 @@ void JsonString::Save(JsonWriter* writer) { // Json Array JsonArray::JsonArray(JsonArray && that) : - Value(ValueKind::Array), vec_{std::move(that.vec_)} {} + Value(ValueKind::kArray), vec_{std::move(that.vec_)} {} Json& JsonArray::operator[](std::string const & key) { LOG(FATAL) << "Object of type " @@ -216,13 +216,13 @@ Json& JsonArray::operator[](int ind) { bool JsonArray::operator==(Value const& rhs) const { if (!IsA(&rhs)) { return false; } - auto& arr = Cast(&rhs)->getArray(); + auto& arr = Cast(&rhs)->GetArray(); return std::equal(arr.cbegin(), arr.cend(), vec_.cbegin()); } Value & JsonArray::operator=(Value const &rhs) { JsonArray const* casted = Cast(&rhs); - vec_ = casted->getArray(); + vec_ = casted->GetArray(); return *this; } @@ -245,12 +245,12 @@ Json& JsonNumber::operator[](int ind) { bool JsonNumber::operator==(Value const& rhs) const { if (!IsA(&rhs)) { return false; } - return std::abs(number_ - Cast(&rhs)->getNumber()) < kRtEps; + return std::abs(number_ - Cast(&rhs)->GetNumber()) < kRtEps; } Value & JsonNumber::operator=(Value const &rhs) { JsonNumber const* casted = Cast(&rhs); - number_ = casted->getNumber(); + number_ = casted->GetNumber(); return *this; } @@ -273,12 +273,12 @@ Json& JsonInteger::operator[](int ind) { bool JsonInteger::operator==(Value const& rhs) const { if (!IsA(&rhs)) { return false; } - return integer_ == Cast(&rhs)->getInteger(); + return integer_ == Cast(&rhs)->GetInteger(); } Value & JsonInteger::operator=(Value const &rhs) { JsonInteger const* casted = Cast(&rhs); - integer_ = casted->getInteger(); + integer_ = casted->GetInteger(); return *this; } @@ -328,12 +328,12 @@ Json& JsonBoolean::operator[](int ind) { bool JsonBoolean::operator==(Value const& rhs) const { if (!IsA(&rhs)) { return false; } - return boolean_ == Cast(&rhs)->getBoolean(); + return boolean_ == Cast(&rhs)->GetBoolean(); } Value & JsonBoolean::operator=(Value const &rhs) { JsonBoolean const* casted = Cast(&rhs); - boolean_ = casted->getBoolean(); + boolean_ = casted->GetBoolean(); return *this; } diff --git a/src/common/observer.h b/src/common/observer.h index 640c4ec47..84996162a 100644 --- a/src/common/observer.h +++ b/src/common/observer.h @@ -36,19 +36,19 @@ namespace xgboost { */ class TrainingObserver { #if defined(XGBOOST_USE_DEBUG_OUTPUT) - bool constexpr static observe_ {true}; + bool constexpr static kObserve {true}; #else - bool constexpr static observe_ {false}; + bool constexpr static kObserve {false}; #endif // defined(XGBOOST_USE_DEBUG_OUTPUT) public: void Update(int32_t iter) const { - if (XGBOOST_EXPECT(!observe_, true)) { return; } + if (XGBOOST_EXPECT(!kObserve, true)) { return; } OBSERVER_PRINT << "Iter: " << iter << OBSERVER_ENDL; } /*\brief Observe tree. */ void Observe(RegTree const& tree) { - if (XGBOOST_EXPECT(!observe_, true)) { return; } + if (XGBOOST_EXPECT(!kObserve, true)) { return; } OBSERVER_PRINT << "Tree:" << OBSERVER_ENDL; Json j_tree {Object()}; tree.SaveModel(&j_tree); @@ -58,7 +58,7 @@ class TrainingObserver { } /*\brief Observe tree. */ void Observe(RegTree const* p_tree) { - if (XGBOOST_EXPECT(!observe_, true)) { return; } + if (XGBOOST_EXPECT(!kObserve, true)) { return; } auto const& tree = *p_tree; this->Observe(tree); } @@ -66,7 +66,7 @@ class TrainingObserver { template void Observe(std::vector const& h_vec, std::string name, size_t n = std::numeric_limits::max()) const { - if (XGBOOST_EXPECT(!observe_, true)) { return; } + if (XGBOOST_EXPECT(!kObserve, true)) { return; } OBSERVER_PRINT << "Procedure: " << name << OBSERVER_ENDL; for (size_t i = 0; i < h_vec.size(); ++i) { @@ -84,14 +84,14 @@ class TrainingObserver { template void Observe(HostDeviceVector const& vec, std::string name, size_t n = std::numeric_limits::max()) const { - if (XGBOOST_EXPECT(!observe_, true)) { return; } + if (XGBOOST_EXPECT(!kObserve, true)) { return; } auto const& h_vec = vec.HostVector(); this->Observe(h_vec, name, n); } template void Observe(HostDeviceVector* vec, std::string name, size_t n = std::numeric_limits::max()) const { - if (XGBOOST_EXPECT(!observe_, true)) { return; } + if (XGBOOST_EXPECT(!kObserve, true)) { return; } this->Observe(*vec, name, n); } @@ -100,14 +100,14 @@ class TrainingObserver { typename std::enable_if< std::is_base_of, Parameter>::value>::type* = nullptr> void Observe(const Parameter &p, std::string name) const { - if (XGBOOST_EXPECT(!observe_, true)) { return; } + if (XGBOOST_EXPECT(!kObserve, true)) { return; } Json obj {toJson(p)}; OBSERVER_PRINT << "Parameter: " << name << ":\n" << obj << OBSERVER_ENDL; } /*\brief Observe parameters provided by users. */ void Observe(Args const& args) const { - if (XGBOOST_EXPECT(!observe_, true)) { return; } + if (XGBOOST_EXPECT(!kObserve, true)) { return; } for (auto kv : args) { OBSERVER_PRINT << kv.first << ": " << kv.second << OBSERVER_NEWLINE; diff --git a/src/common/probability_distribution.h b/src/common/probability_distribution.h index ccf3bb96c..66d6a3d44 100644 --- a/src/common/probability_distribution.h +++ b/src/common/probability_distribution.h @@ -59,6 +59,7 @@ class ProbabilityDistribution { * \return Reference to the newly created probability distribution object */ static ProbabilityDistribution* Create(ProbabilityDistributionType dist); + virtual ~ProbabilityDistribution() = default; }; /*! \brief The (standard) normal distribution */ diff --git a/src/common/row_set.h b/src/common/row_set.h index 179e07d50..25f7c739d 100644 --- a/src/common/row_set.h +++ b/src/common/row_set.h @@ -89,6 +89,8 @@ class RowSetCollection { const size_t* end = dmlc::BeginPtr(row_indices_) + row_indices_.size(); elem_of_each_node_.emplace_back(Elem(begin, end, 0)); } + + std::vector* Data() { return &row_indices_; } // split rowset into two inline void AddSplit(unsigned node_id, unsigned left_node_id, @@ -116,10 +118,9 @@ class RowSetCollection { elem_of_each_node_[node_id] = Elem(nullptr, nullptr, -1); } + private: // stores the row indexes in the set std::vector row_indices_; - - private: // vector: node_id -> elements std::vector elem_of_each_node_; }; @@ -151,12 +152,12 @@ class PartitionBuilder { common::Span GetLeftBuffer(int nid, size_t begin, size_t end) { const size_t task_idx = GetTaskIdx(nid, begin); - return { mem_blocks_.at(task_idx).left(), end - begin }; + return { mem_blocks_.at(task_idx).Left(), end - begin }; } common::Span GetRightBuffer(int nid, size_t begin, size_t end) { const size_t task_idx = GetTaskIdx(nid, begin); - return { mem_blocks_.at(task_idx).right(), end - begin }; + return { mem_blocks_.at(task_idx).Right(), end - begin }; } void SetNLeftElems(int nid, size_t begin, size_t end, size_t n_left) { @@ -202,8 +203,8 @@ class PartitionBuilder { size_t* left_result = rows_indexes + mem_blocks_[task_idx].n_offset_left; size_t* right_result = rows_indexes + mem_blocks_[task_idx].n_offset_right; - const size_t* left = mem_blocks_[task_idx].left(); - const size_t* right = mem_blocks_[task_idx].right(); + const size_t* left = mem_blocks_[task_idx].Left(); + const size_t* right = mem_blocks_[task_idx].Right(); std::copy_n(left, mem_blocks_[task_idx].n_left, left_result); std::copy_n(right, mem_blocks_[task_idx].n_right, right_result); @@ -221,11 +222,11 @@ class PartitionBuilder { size_t n_offset_left; size_t n_offset_right; - size_t* left() { + size_t* Left() { return &left_data_[0]; } - size_t* right() { + size_t* Right() { return &right_data_[0]; } private: diff --git a/src/common/timer.cc b/src/common/timer.cc index b9f9f200d..2cc140496 100644 --- a/src/common/timer.cc +++ b/src/common/timer.cc @@ -15,13 +15,13 @@ namespace common { void Monitor::Start(std::string const &name) { if (ConsoleLogger::ShouldLog(ConsoleLogger::LV::kDebug)) { - statistics_map[name].timer.Start(); + statistics_map_[name].timer.Start(); } } void Monitor::Stop(const std::string &name) { if (ConsoleLogger::ShouldLog(ConsoleLogger::LV::kDebug)) { - auto &stats = statistics_map[name]; + auto &stats = statistics_map_[name]; stats.timer.Stop(); stats.count++; } @@ -40,7 +40,7 @@ std::vector Monitor::CollectFromOtherRanks() const { j_statistic["statistic"] = Object(); auto& statistic = j_statistic["statistic"]; - for (auto const& kv : statistics_map) { + for (auto const& kv : statistics_map_) { statistic[kv.first] = Object(); auto& j_pair = statistic[kv.first]; j_pair["count"] = Integer(kv.second.count); @@ -105,7 +105,7 @@ void Monitor::Print() const { auto world = this->CollectFromOtherRanks(); // rank zero is in charge of printing if (rabit::GetRank() == 0) { - LOG(CONSOLE) << "======== Monitor: " << label << " ========"; + LOG(CONSOLE) << "======== Monitor: " << label_ << " ========"; for (size_t i = 0; i < world.size(); ++i) { LOG(CONSOLE) << "From rank: " << i << ": " << std::endl; auto const& statistic = world[i]; @@ -114,12 +114,12 @@ void Monitor::Print() const { } } else { StatMap stat_map; - for (auto const& kv : statistics_map) { + for (auto const& kv : statistics_map_) { stat_map[kv.first] = std::make_pair( kv.second.count, std::chrono::duration_cast( kv.second.timer.elapsed).count()); } - LOG(CONSOLE) << "======== Monitor: " << label << " ========"; + LOG(CONSOLE) << "======== Monitor: " << label_ << " ========"; this->PrintStatistics(stat_map); } } diff --git a/src/common/timer.cu b/src/common/timer.cu index 5cb72ddba..8b8e54bc3 100644 --- a/src/common/timer.cu +++ b/src/common/timer.cu @@ -16,7 +16,7 @@ namespace common { void Monitor::StartCuda(const std::string& name) { if (ConsoleLogger::ShouldLog(ConsoleLogger::LV::kDebug)) { - auto &stats = statistics_map[name]; + auto &stats = statistics_map_[name]; stats.timer.Start(); #if defined(XGBOOST_USE_NVTX) stats.nvtx_id = nvtxRangeStartA(name.c_str()); @@ -26,7 +26,7 @@ void Monitor::StartCuda(const std::string& name) { void Monitor::StopCuda(const std::string& name) { if (ConsoleLogger::ShouldLog(ConsoleLogger::LV::kDebug)) { - auto &stats = statistics_map[name]; + auto &stats = statistics_map_[name]; stats.timer.Stop(); stats.count++; #if defined(XGBOOST_USE_NVTX) diff --git a/src/common/timer.h b/src/common/timer.h index 86c3a3cef..bc71dca8e 100644 --- a/src/common/timer.h +++ b/src/common/timer.h @@ -55,16 +55,16 @@ struct Monitor { // from left to right, > using StatMap = std::map>; - std::string label = ""; - std::map statistics_map; - Timer self_timer; + std::string label_ = ""; + std::map statistics_map_; + Timer self_timer_; /*! \brief Collect time statistics across all workers. */ std::vector CollectFromOtherRanks() const; void PrintStatistics(StatMap const& statistics) const; public: - Monitor() { self_timer.Start(); } + Monitor() { self_timer_.Start(); } /*\brief Print statistics info during destruction. * * Please note that this may not work, as with distributed frameworks like Dask, the @@ -73,13 +73,13 @@ struct Monitor { */ ~Monitor() { this->Print(); - self_timer.Stop(); + self_timer_.Stop(); } /*! \brief Print all the statistics. */ void Print() const; - void Init(std::string label) { this->label = label; } + void Init(std::string label) { this->label_ = label; } void Start(const std::string &name); void Stop(const std::string &name); void StartCuda(const std::string &name); diff --git a/src/common/transform.h b/src/common/transform.h index d204ebf86..fa2d0d379 100644 --- a/src/common/transform.h +++ b/src/common/transform.h @@ -133,8 +133,9 @@ class Transform { template ::type* = nullptr, typename... HDV> void LaunchCUDA(Functor _func, HDV*... _vectors) const { - if (shard_) + if (shard_) { UnpackShard(device_, _vectors...); + } size_t range_size = *range_.end() - *range_.begin(); diff --git a/src/data/array_interface.h b/src/data/array_interface.h index 6502e16d0..1c07f2bb0 100644 --- a/src/data/array_interface.h +++ b/src/data/array_interface.h @@ -41,7 +41,7 @@ struct ArrayInterfaceErrors { static char const* Version() { return "Only version 1 of `__cuda_array_interface__' is supported."; } - static char const* ofType(std::string const& type) { + static char const* OfType(std::string const& type) { static std::string str; str.clear(); str += " should be of "; @@ -229,9 +229,6 @@ class ArrayInterfaceHandler { // A view over __array_interface__ class ArrayInterface { - using mask_type = unsigned char; - using index_type = int32_t; - public: ArrayInterface() = default; explicit ArrayInterface(std::map const& column) { diff --git a/src/data/device_adapter.cuh b/src/data/device_adapter.cuh index e43971a35..80b777aa8 100644 --- a/src/data/device_adapter.cuh +++ b/src/data/device_adapter.cuh @@ -40,8 +40,8 @@ class CudfAdapterBatch : public detail::NoMetaInfo { common::Span column_ptr, size_t num_elements) : columns_(columns), column_ptr_(column_ptr), - num_elements(num_elements) {} - size_t Size() const { return num_elements; } + num_elements_(num_elements) {} + size_t Size() const { return num_elements_; } __device__ COOTuple GetElement(size_t idx) const { size_t column_idx = dh::UpperBound(column_ptr_.data(), column_ptr_.size(), idx) - 1; @@ -50,7 +50,7 @@ class CudfAdapterBatch : public detail::NoMetaInfo { float value = column.valid.Data() == nullptr || column.valid.Check(row_idx) ? column.GetElement(row_idx) : std::numeric_limits::quiet_NaN(); - return COOTuple(row_idx, column_idx, value); + return {row_idx, column_idx, value}; } __device__ float GetValue(size_t ridx, bst_feature_t fidx) const { auto const& column = columns_[fidx]; @@ -63,7 +63,7 @@ class CudfAdapterBatch : public detail::NoMetaInfo { private: common::Span columns_; common::Span column_ptr_; - size_t num_elements; + size_t num_elements_; }; /*! @@ -146,10 +146,10 @@ class CudfAdapter : public detail::SingleBatchDataIter { } columns_ = columns; column_ptr_ = column_ptr; - batch = CudfAdapterBatch(dh::ToSpan(columns_), dh::ToSpan(column_ptr_), + batch_ = CudfAdapterBatch(dh::ToSpan(columns_), dh::ToSpan(column_ptr_), column_ptr.back()); } - const CudfAdapterBatch& Value() const override { return batch; } + const CudfAdapterBatch& Value() const override { return batch_; } size_t NumRows() const { return num_rows_; } size_t NumColumns() const { return columns_.size(); } @@ -159,7 +159,7 @@ class CudfAdapter : public detail::SingleBatchDataIter { bool IsRowMajor() { return false; } private: - CudfAdapterBatch batch; + CudfAdapterBatch batch_; dh::device_vector columns_; dh::device_vector column_ptr_; // Exclusive scan of column sizes size_t num_rows_{0}; @@ -169,8 +169,8 @@ class CudfAdapter : public detail::SingleBatchDataIter { class CupyAdapterBatch : public detail::NoMetaInfo { public: CupyAdapterBatch() = default; - CupyAdapterBatch(ArrayInterface array_interface) - : array_interface_(array_interface) {} + explicit CupyAdapterBatch(ArrayInterface array_interface) + : array_interface_(std::move(array_interface)) {} size_t Size() const { return array_interface_.num_rows * array_interface_.num_cols; } @@ -181,7 +181,7 @@ class CupyAdapterBatch : public detail::NoMetaInfo { array_interface_.valid.Check(row_idx) ? array_interface_.GetElement(idx) : std::numeric_limits::quiet_NaN(); - return COOTuple(row_idx, column_idx, value); + return {row_idx, column_idx, value}; } private: @@ -193,22 +193,22 @@ class CupyAdapter : public detail::SingleBatchDataIter { explicit CupyAdapter(std::string cuda_interface_str) { Json json_array_interface = Json::Load({cuda_interface_str.c_str(), cuda_interface_str.size()}); - array_interface = ArrayInterface(get(json_array_interface)); - device_idx_ = dh::CudaGetPointerDevice(array_interface.data); + array_interface_ = ArrayInterface(get(json_array_interface)); + device_idx_ = dh::CudaGetPointerDevice(array_interface_.data); CHECK_NE(device_idx_, -1); - batch = CupyAdapterBatch(array_interface); + batch_ = CupyAdapterBatch(array_interface_); } - const CupyAdapterBatch& Value() const override { return batch; } + const CupyAdapterBatch& Value() const override { return batch_; } - size_t NumRows() const { return array_interface.num_rows; } - size_t NumColumns() const { return array_interface.num_cols; } + size_t NumRows() const { return array_interface_.num_rows; } + size_t NumColumns() const { return array_interface_.num_cols; } size_t DeviceIdx() const { return device_idx_; } bool IsRowMajor() { return true; } private: - ArrayInterface array_interface; - CupyAdapterBatch batch; + ArrayInterface array_interface_; + CupyAdapterBatch batch_; int device_idx_; }; diff --git a/src/data/device_dmatrix.cu b/src/data/device_dmatrix.cu index 0a2eaf45a..a99e3a03c 100644 --- a/src/data/device_dmatrix.cu +++ b/src/data/device_dmatrix.cu @@ -46,12 +46,12 @@ template struct WriteCompressedEllpackFunctor { WriteCompressedEllpackFunctor(common::CompressedByteT* buffer, const common::CompressedBufferWriter& writer, - const AdapterBatchT& batch, + AdapterBatchT batch, EllpackDeviceAccessor accessor, const IsValidFunctor& is_valid) : d_buffer(buffer), writer(writer), - batch(batch), + batch(std::move(batch)), accessor(std::move(accessor)), is_valid(is_valid) {} @@ -210,10 +210,10 @@ DeviceDMatrix::DeviceDMatrix(AdapterT* adapter, float missing, int nthread, int GetRowCounts(batch, row_counts_span, adapter->DeviceIdx(), missing); dh::XGBCachingDeviceAllocator alloc; - info.num_nonzero_ = thrust::reduce(thrust::cuda::par(alloc), + info_.num_nonzero_ = thrust::reduce(thrust::cuda::par(alloc), row_counts.begin(), row_counts.end()); - info.num_col_ = adapter->NumColumns(); - info.num_row_ = adapter->NumRows(); + info_.num_col_ = adapter->NumColumns(); + info_.num_row_ = adapter->NumRows(); ellpack_page_.reset(new EllpackPage()); *ellpack_page_->Impl() = EllpackPageImpl(adapter->DeviceIdx(), cuts, this->IsDense(), row_stride, @@ -228,7 +228,7 @@ DeviceDMatrix::DeviceDMatrix(AdapterT* adapter, float missing, int nthread, int WriteNullValues(ellpack_page_->Impl(), adapter->DeviceIdx(), row_counts_span); // Synchronise worker columns - rabit::Allreduce(&info.num_col_, 1); + rabit::Allreduce(&info_.num_col_, 1); } template DeviceDMatrix::DeviceDMatrix(CudfAdapter* adapter, float missing, int nthread, int max_bin); diff --git a/src/data/device_dmatrix.h b/src/data/device_dmatrix.h index 162f2a41e..2442175a7 100644 --- a/src/data/device_dmatrix.h +++ b/src/data/device_dmatrix.h @@ -23,9 +23,9 @@ class DeviceDMatrix : public DMatrix { template explicit DeviceDMatrix(AdapterT* adapter, float missing, int nthread, int max_bin); - MetaInfo& Info() override { return info; } + MetaInfo& Info() override { return info_; } - const MetaInfo& Info() const override { return info; } + const MetaInfo& Info() const override { return info_; } bool SingleColBlock() const override { return true; } @@ -51,7 +51,7 @@ class DeviceDMatrix : public DMatrix { return BatchSet(begin_iter); } - MetaInfo info; + MetaInfo info_; // source data pointer. std::unique_ptr ellpack_page_; }; diff --git a/src/data/ellpack_page.cuh b/src/data/ellpack_page.cuh index 93773415a..98ed3587a 100644 --- a/src/data/ellpack_page.cuh +++ b/src/data/ellpack_page.cuh @@ -120,7 +120,7 @@ struct EllpackDeviceAccessor { * not found). */ XGBOOST_DEVICE size_t NumSymbols() const { return gidx_fvalue_map.size() + 1; } - size_t NullValue() const { return gidx_fvalue_map.size(); } + XGBOOST_DEVICE size_t NullValue() const { return gidx_fvalue_map.size(); } XGBOOST_DEVICE size_t NumBins() const { return gidx_fvalue_map.size(); } @@ -185,6 +185,9 @@ class EllpackPageImpl { base_rowid = row_id; } + common::HistogramCuts& Cuts() { return cuts_; } + common::HistogramCuts const& Cuts() const { return cuts_; } + /*! \return Estimation of memory cost of this page. */ static size_t MemCostBytes(size_t num_rows, size_t row_stride, const common::HistogramCuts&cuts) ; @@ -220,8 +223,9 @@ public: size_t n_rows{}; /*! \brief global index of histogram, which is stored in ELLPack format. */ HostDeviceVector gidx_buffer; + + private: common::HistogramCuts cuts_; -private: common::Monitor monitor_; }; diff --git a/src/data/ellpack_page_raw_format.cu b/src/data/ellpack_page_raw_format.cu index 147d8fb4d..d4caf37e2 100644 --- a/src/data/ellpack_page_raw_format.cu +++ b/src/data/ellpack_page_raw_format.cu @@ -17,9 +17,9 @@ class EllpackPageRawFormat : public SparsePageFormat { public: bool Read(EllpackPage* page, dmlc::SeekStream* fi) override { auto* impl = page->Impl(); - fi->Read(&impl->cuts_.cut_values_.HostVector()); - fi->Read(&impl->cuts_.cut_ptrs_.HostVector()); - fi->Read(&impl->cuts_.min_vals_.HostVector()); + fi->Read(&impl->Cuts().cut_values_.HostVector()); + fi->Read(&impl->Cuts().cut_ptrs_.HostVector()); + fi->Read(&impl->Cuts().min_vals_.HostVector()); fi->Read(&impl->n_rows); fi->Read(&impl->is_dense); fi->Read(&impl->row_stride); @@ -38,9 +38,9 @@ class EllpackPageRawFormat : public SparsePageFormat { void Write(const EllpackPage& page, dmlc::Stream* fo) override { auto* impl = page.Impl(); - fo->Write(impl->cuts_.cut_values_.ConstHostVector()); - fo->Write(impl->cuts_.cut_ptrs_.ConstHostVector()); - fo->Write(impl->cuts_.min_vals_.ConstHostVector()); + fo->Write(impl->Cuts().cut_values_.ConstHostVector()); + fo->Write(impl->Cuts().cut_ptrs_.ConstHostVector()); + fo->Write(impl->Cuts().min_vals_.ConstHostVector()); fo->Write(impl->n_rows); fo->Write(impl->is_dense); fo->Write(impl->row_stride); diff --git a/src/data/simple_dmatrix.cc b/src/data/simple_dmatrix.cc index 833949eab..64637695c 100644 --- a/src/data/simple_dmatrix.cc +++ b/src/data/simple_dmatrix.cc @@ -12,9 +12,9 @@ namespace xgboost { namespace data { -MetaInfo& SimpleDMatrix::Info() { return info; } +MetaInfo& SimpleDMatrix::Info() { return info_; } -const MetaInfo& SimpleDMatrix::Info() const { return info; } +const MetaInfo& SimpleDMatrix::Info() const { return info_; } BatchSet SimpleDMatrix::GetRowBatches() { // since csr is the default data structure so `source_` is always available. @@ -26,7 +26,7 @@ BatchSet SimpleDMatrix::GetRowBatches() { BatchSet SimpleDMatrix::GetColumnBatches() { // column page doesn't exist, generate it if (!column_page_) { - column_page_.reset(new CSCPage(sparse_page_.GetTranspose(info.num_col_))); + column_page_.reset(new CSCPage(sparse_page_.GetTranspose(info_.num_col_))); } auto begin_iter = BatchIterator(new SimpleBatchIteratorImpl(column_page_.get())); @@ -37,7 +37,7 @@ BatchSet SimpleDMatrix::GetSortedColumnBatches() { // Sorted column page doesn't exist, generate it if (!sorted_column_page_) { sorted_column_page_.reset( - new SortedCSCPage(sparse_page_.GetTranspose(info.num_col_))); + new SortedCSCPage(sparse_page_.GetTranspose(info_.num_col_))); sorted_column_page_->SortRows(); } auto begin_iter = BatchIterator( @@ -85,17 +85,17 @@ SimpleDMatrix::SimpleDMatrix(AdapterT* adapter, float missing, int nthread) { inferred_num_columns = std::max(batch_max_columns, inferred_num_columns); // Append meta information if available if (batch.Labels() != nullptr) { - auto& labels = info.labels_.HostVector(); + auto& labels = info_.labels_.HostVector(); labels.insert(labels.end(), batch.Labels(), batch.Labels() + batch.Size()); } if (batch.Weights() != nullptr) { - auto& weights = info.weights_.HostVector(); + auto& weights = info_.weights_.HostVector(); weights.insert(weights.end(), batch.Weights(), batch.Weights() + batch.Size()); } if (batch.BaseMargin() != nullptr) { - auto& base_margin = info.base_margin_.HostVector(); + auto& base_margin = info_.base_margin_.HostVector(); base_margin.insert(base_margin.end(), batch.BaseMargin(), batch.BaseMargin() + batch.Size()); } @@ -105,7 +105,7 @@ SimpleDMatrix::SimpleDMatrix(AdapterT* adapter, float missing, int nthread) { for (size_t i = 0; i < batch.Size(); ++i) { const uint64_t cur_group_id = batch.Qid()[i]; if (last_group_id == default_max || last_group_id != cur_group_id) { - info.group_ptr_.push_back(group_size); + info_.group_ptr_.push_back(group_size); } last_group_id = cur_group_id; ++group_size; @@ -114,22 +114,22 @@ SimpleDMatrix::SimpleDMatrix(AdapterT* adapter, float missing, int nthread) { } if (last_group_id != default_max) { - if (group_size > info.group_ptr_.back()) { - info.group_ptr_.push_back(group_size); + if (group_size > info_.group_ptr_.back()) { + info_.group_ptr_.push_back(group_size); } } // Deal with empty rows/columns if necessary if (adapter->NumColumns() == kAdapterUnknownSize) { - info.num_col_ = inferred_num_columns; + info_.num_col_ = inferred_num_columns; } else { - info.num_col_ = adapter->NumColumns(); + info_.num_col_ = adapter->NumColumns(); } // Synchronise worker columns - rabit::Allreduce(&info.num_col_, 1); + rabit::Allreduce(&info_.num_col_, 1); if (adapter->NumRows() == kAdapterUnknownSize) { - info.num_row_ = offset_vec.size() - 1; + info_.num_row_ = offset_vec.size() - 1; } else { if (offset_vec.empty()) { offset_vec.emplace_back(0); @@ -138,9 +138,9 @@ SimpleDMatrix::SimpleDMatrix(AdapterT* adapter, float missing, int nthread) { while (offset_vec.size() - 1 < adapter->NumRows()) { offset_vec.emplace_back(offset_vec.back()); } - info.num_row_ = adapter->NumRows(); + info_.num_row_ = adapter->NumRows(); } - info.num_nonzero_ = data_vec.size(); + info_.num_nonzero_ = data_vec.size(); omp_set_num_threads(nthread_original); } @@ -149,7 +149,7 @@ SimpleDMatrix::SimpleDMatrix(dmlc::Stream* in_stream) { CHECK(in_stream->Read(&tmagic, sizeof(tmagic)) == sizeof(tmagic)) << "invalid input file format"; CHECK_EQ(tmagic, kMagic) << "invalid format, magic number mismatch"; - info.LoadBinary(in_stream); + info_.LoadBinary(in_stream); in_stream->Read(&sparse_page_.offset.HostVector()); in_stream->Read(&sparse_page_.data.HostVector()); } @@ -158,7 +158,7 @@ void SimpleDMatrix::SaveToLocalFile(const std::string& fname) { std::unique_ptr fo(dmlc::Stream::Create(fname.c_str(), "w")); int tmagic = kMagic; fo->Write(&tmagic, sizeof(tmagic)); - info.SaveBinary(fo.get()); + info_.SaveBinary(fo.get()); fo->Write(sparse_page_.offset.HostVector()); fo->Write(sparse_page_.data.HostVector()); } diff --git a/src/data/simple_dmatrix.cu b/src/data/simple_dmatrix.cu index 63c96bc67..f0836e81e 100644 --- a/src/data/simple_dmatrix.cu +++ b/src/data/simple_dmatrix.cu @@ -113,8 +113,8 @@ SimpleDMatrix::SimpleDMatrix(AdapterT* adapter, float missing, int nthread) { sparse_page_.offset.Resize(adapter->NumRows() + 1); auto s_offset = sparse_page_.offset.DeviceSpan(); CountRowOffsets(batch, s_offset, adapter->DeviceIdx(), missing); - info.num_nonzero_ = sparse_page_.offset.HostVector().back(); - sparse_page_.data.Resize(info.num_nonzero_); + info_.num_nonzero_ = sparse_page_.offset.HostVector().back(); + sparse_page_.data.Resize(info_.num_nonzero_); if (adapter->IsRowMajor()) { CopyDataRowMajor(adapter, sparse_page_.data.DeviceSpan(), adapter->DeviceIdx(), missing, s_offset); @@ -123,10 +123,10 @@ SimpleDMatrix::SimpleDMatrix(AdapterT* adapter, float missing, int nthread) { adapter->DeviceIdx(), missing, s_offset); } - info.num_col_ = adapter->NumColumns(); - info.num_row_ = adapter->NumRows(); + info_.num_col_ = adapter->NumColumns(); + info_.num_row_ = adapter->NumRows(); // Synchronise worker columns - rabit::Allreduce(&info.num_col_, 1); + rabit::Allreduce(&info_.num_col_, 1); } template SimpleDMatrix::SimpleDMatrix(CudfAdapter* adapter, float missing, diff --git a/src/data/simple_dmatrix.h b/src/data/simple_dmatrix.h index daf7aac13..e0b41a4ef 100644 --- a/src/data/simple_dmatrix.h +++ b/src/data/simple_dmatrix.h @@ -42,7 +42,7 @@ class SimpleDMatrix : public DMatrix { BatchSet GetSortedColumnBatches() override; BatchSet GetEllpackBatches(const BatchParam& param) override; - MetaInfo info; + MetaInfo info_; SparsePage sparse_page_; // Primary storage type std::unique_ptr column_page_; std::unique_ptr sorted_column_page_; diff --git a/src/data/sparse_page_source.h b/src/data/sparse_page_source.h index 63466493e..c7074c092 100644 --- a/src/data/sparse_page_source.h +++ b/src/data/sparse_page_source.h @@ -27,7 +27,7 @@ #include "../common/common.h" #include -namespace { +namespace detail { // Split a cache info string with delimiter ':' // If cache info string contains drive letter (e.g. C:), exclude it before splitting @@ -46,7 +46,7 @@ GetCacheShards(const std::string& cache_info) { return xgboost::common::Split(cache_info, ':'); } -} // anonymous namespace +} // namespace detail namespace xgboost { namespace data { @@ -100,7 +100,7 @@ struct CacheInfo { inline CacheInfo ParseCacheInfo(const std::string& cache_info, const std::string& page_type) { CacheInfo info; - std::vector cache_shards = GetCacheShards(cache_info); + std::vector cache_shards = ::detail::GetCacheShards(cache_info); CHECK_NE(cache_shards.size(), 0U); // read in the info files. info.name_info = cache_shards[0]; diff --git a/src/gbm/gblinear.cc b/src/gbm/gblinear.cc index 3fff06d99..8e4528d28 100644 --- a/src/gbm/gblinear.cc +++ b/src/gbm/gblinear.cc @@ -53,8 +53,8 @@ class GBLinear : public GradientBooster { public: explicit GBLinear(LearnerModelParam const* learner_model_param) : learner_model_param_{learner_model_param}, - model_{learner_model_param_}, - previous_model_{learner_model_param_}, + model_{learner_model_param}, + previous_model_{learner_model_param}, sum_instance_weight_(0), sum_weight_complete_(false), is_converged_(false) {} @@ -95,14 +95,14 @@ class GBLinear : public GradientBooster { void LoadConfig(Json const& in) override { CHECK_EQ(get(in["name"]), "gblinear"); - fromJson(in["gblinear_train_param"], ¶m_); + FromJson(in["gblinear_train_param"], ¶m_); updater_.reset(LinearUpdater::Create(param_.updater, generic_param_)); this->updater_->LoadConfig(in["updater"]); } void SaveConfig(Json* p_out) const override { auto& out = *p_out; out["name"] = String{"gblinear"}; - out["gblinear_train_param"] = toJson(param_); + out["gblinear_train_param"] = ToJson(param_); out["updater"] = Object(); auto& j_updater = out["updater"]; @@ -140,7 +140,7 @@ class GBLinear : public GradientBooster { void PredictInstance(const SparsePage::Inst &inst, std::vector *out_preds, unsigned ntree_limit) override { - const int ngroup = model_.learner_model_param_->num_output_group; + const int ngroup = model_.learner_model_param->num_output_group; for (int gid = 0; gid < ngroup; ++gid) { this->Pred(inst, dmlc::BeginPtr(*out_preds), gid, learner_model_param_->base_score); @@ -161,8 +161,8 @@ class GBLinear : public GradientBooster { CHECK_EQ(ntree_limit, 0U) << "GBLinear::PredictContribution: ntrees is only valid for gbtree predictor"; const auto& base_margin = p_fmat->Info().base_margin_.ConstHostVector(); - const int ngroup = model_.learner_model_param_->num_output_group; - const size_t ncolumns = model_.learner_model_param_->num_feature + 1; + const int ngroup = model_.learner_model_param->num_output_group; + const size_t ncolumns = model_.learner_model_param->num_feature + 1; // allocate space for (#features + bias) times #groups times #rows std::vector& contribs = *out_contribs; contribs.resize(p_fmat->Info().num_row_ * ncolumns * ngroup); @@ -181,11 +181,11 @@ class GBLinear : public GradientBooster { bst_float *p_contribs = &contribs[(row_idx * ngroup + gid) * ncolumns]; // calculate linear terms' contributions for (auto& ins : inst) { - if (ins.index >= model_.learner_model_param_->num_feature) continue; + if (ins.index >= model_.learner_model_param->num_feature) continue; p_contribs[ins.index] = ins.fvalue * model_[ins.index][gid]; } // add base margin to BIAS - p_contribs[ncolumns - 1] = model_.bias()[gid] + + p_contribs[ncolumns - 1] = model_.Bias()[gid] + ((base_margin.size() != 0) ? base_margin[row_idx * ngroup + gid] : learner_model_param_->base_score); } @@ -199,10 +199,10 @@ class GBLinear : public GradientBooster { std::vector& contribs = *out_contribs; // linear models have no interaction effects - const size_t nelements = model_.learner_model_param_->num_feature * - model_.learner_model_param_->num_feature; + const size_t nelements = model_.learner_model_param->num_feature * + model_.learner_model_param->num_feature; contribs.resize(p_fmat->Info().num_row_ * nelements * - model_.learner_model_param_->num_output_group); + model_.learner_model_param->num_output_group); std::fill(contribs.begin(), contribs.end(), 0); } @@ -228,7 +228,7 @@ class GBLinear : public GradientBooster { std::vector &preds = *out_preds; const auto& base_margin = p_fmat->Info().base_margin_.ConstHostVector(); // start collecting the prediction - const int ngroup = model_.learner_model_param_->num_output_group; + const int ngroup = model_.learner_model_param->num_output_group; preds.resize(p_fmat->Info().num_row_ * ngroup); for (const auto &batch : p_fmat->GetBatches()) { // output convention: nrow * k, where nrow is number of rows @@ -283,9 +283,9 @@ class GBLinear : public GradientBooster { void Pred(const SparsePage::Inst &inst, bst_float *preds, int gid, bst_float base) { - bst_float psum = model_.bias()[gid] + base; + bst_float psum = model_.Bias()[gid] + base; for (const auto& ins : inst) { - if (ins.index >= model_.learner_model_param_->num_feature) continue; + if (ins.index >= model_.learner_model_param->num_feature) continue; psum += ins.fvalue * model_[ins.index][gid]; } preds[gid] = psum; diff --git a/src/gbm/gblinear_model.h b/src/gbm/gblinear_model.h index 1564ffedd..f2d0d9a86 100644 --- a/src/gbm/gblinear_model.h +++ b/src/gbm/gblinear_model.h @@ -41,14 +41,14 @@ struct DeprecatedGBLinearModelParam : public dmlc::Parameternum_feature + 1) * - learner_model_param_->num_output_group); + weight.resize((learner_model_param->num_feature + 1) * + learner_model_param->num_output_group); std::fill(weight.begin(), weight.end(), 0.0f); } @@ -69,52 +69,54 @@ class GBLinearModel : public Model { // save the model to file void Save(dmlc::Stream *fo) const { - fo->Write(¶m, sizeof(param)); + fo->Write(¶m_, sizeof(param_)); fo->Write(weight); } // load model from file void Load(dmlc::Stream *fi) { - CHECK_EQ(fi->Read(¶m, sizeof(param)), sizeof(param)); + CHECK_EQ(fi->Read(¶m_, sizeof(param_)), sizeof(param_)); fi->Read(&weight); } // model bias - inline bst_float *bias() { - return &weight[learner_model_param_->num_feature * - learner_model_param_->num_output_group]; + inline bst_float *Bias() { + return &weight[learner_model_param->num_feature * + learner_model_param->num_output_group]; } - inline const bst_float *bias() const { - return &weight[learner_model_param_->num_feature * - learner_model_param_->num_output_group]; + inline const bst_float *Bias() const { + return &weight[learner_model_param->num_feature * + learner_model_param->num_output_group]; } // get i-th weight inline bst_float *operator[](size_t i) { - return &weight[i * learner_model_param_->num_output_group]; + return &weight[i * learner_model_param->num_output_group]; } inline const bst_float *operator[](size_t i) const { - return &weight[i * learner_model_param_->num_output_group]; + return &weight[i * learner_model_param->num_output_group]; } std::vector DumpModel(const FeatureMap &fmap, bool with_stats, std::string format) const { - const int ngroup = learner_model_param_->num_output_group; - const unsigned nfeature = learner_model_param_->num_feature; + const int ngroup = learner_model_param->num_output_group; + const unsigned nfeature = learner_model_param->num_feature; std::stringstream fo(""); if (format == "json") { fo << " { \"bias\": [" << std::endl; for (int gid = 0; gid < ngroup; ++gid) { - if (gid != 0) + if (gid != 0) { fo << "," << std::endl; - fo << " " << this->bias()[gid]; + } + fo << " " << this->Bias()[gid]; } fo << std::endl << " ]," << std::endl << " \"weight\": [" << std::endl; for (unsigned i = 0; i < nfeature; ++i) { for (int gid = 0; gid < ngroup; ++gid) { - if (i != 0 || gid != 0) + if (i != 0 || gid != 0) { fo << "," << std::endl; + } fo << " " << (*this)[i][gid]; } } @@ -122,7 +124,7 @@ class GBLinearModel : public Model { } else { fo << "bias:\n"; for (int gid = 0; gid < ngroup; ++gid) { - fo << this->bias()[gid] << std::endl; + fo << this->Bias()[gid] << std::endl; } fo << "weight:\n"; for (unsigned i = 0; i < nfeature; ++i) { diff --git a/src/gbm/gbtree.cc b/src/gbm/gbtree.cc index fe8fa98d6..9c1df87da 100644 --- a/src/gbm/gbtree.cc +++ b/src/gbm/gbtree.cc @@ -186,7 +186,7 @@ void GBTree::DoBoost(DMatrix* p_fmat, HostDeviceVector* in_gpair, PredictionCacheEntry* predt) { std::vector > > new_trees; - const int ngroup = model_.learner_model_param_->num_output_group; + const int ngroup = model_.learner_model_param->num_output_group; ConfigureWithKnownData(this->cfg_, p_fmat); monitor_.Start("BoostNewTrees"); CHECK_NE(ngroup, 0); @@ -300,17 +300,17 @@ void GBTree::CommitModel(std::vector>>&& ne PredictionCacheEntry* predts) { monitor_.Start("CommitModel"); int num_new_trees = 0; - for (uint32_t gid = 0; gid < model_.learner_model_param_->num_output_group; ++gid) { + for (uint32_t gid = 0; gid < model_.learner_model_param->num_output_group; ++gid) { num_new_trees += new_trees[gid].size(); model_.CommitModel(std::move(new_trees[gid]), gid); } auto* out = &predts->predictions; - if (model_.learner_model_param_->num_output_group == 1 && + if (model_.learner_model_param->num_output_group == 1 && updaters_.size() > 0 && num_new_trees == 1 && out->Size() > 0 && updaters_.back()->UpdatePredictionCache(m, out)) { - auto delta = num_new_trees / model_.learner_model_param_->num_output_group; + auto delta = num_new_trees / model_.learner_model_param->num_output_group; predts->Update(delta); } monitor_.Stop("CommitModel"); @@ -318,7 +318,7 @@ void GBTree::CommitModel(std::vector>>&& ne void GBTree::LoadConfig(Json const& in) { CHECK_EQ(get(in["name"]), "gbtree"); - fromJson(in["gbtree_train_param"], &tparam_); + FromJson(in["gbtree_train_param"], &tparam_); int32_t const n_gpus = xgboost::common::AllVisibleGPUs(); if (n_gpus == 0 && tparam_.predictor == PredictorType::kGPUPredictor) { LOG(WARNING) @@ -347,7 +347,7 @@ void GBTree::LoadConfig(Json const& in) { void GBTree::SaveConfig(Json* p_out) const { auto& out = *p_out; out["name"] = String("gbtree"); - out["gbtree_train_param"] = toJson(tparam_); + out["gbtree_train_param"] = ToJson(tparam_); out["updater"] = Object(); auto& j_updaters = out["updater"]; @@ -495,7 +495,7 @@ class Dart : public GBTree { CHECK_EQ(get(in["name"]), "dart"); auto const& gbtree = in["gbtree"]; GBTree::LoadConfig(gbtree); - fromJson(in["dart_train_param"], &dparam_); + FromJson(in["dart_train_param"], &dparam_); } void SaveConfig(Json* p_out) const override { auto& out = *p_out; @@ -503,7 +503,7 @@ class Dart : public GBTree { out["gbtree"] = Object(); auto& gbtree = out["gbtree"]; GBTree::SaveConfig(&gbtree); - out["dart_train_param"] = toJson(dparam_); + out["dart_train_param"] = ToJson(dparam_); } void PredictBatch(DMatrix* p_fmat, @@ -511,7 +511,7 @@ class Dart : public GBTree { bool training, unsigned ntree_limit) override { DropTrees(training); - int num_group = model_.learner_model_param_->num_output_group; + int num_group = model_.learner_model_param->num_output_group; ntree_limit *= num_group; if (ntree_limit == 0 || ntree_limit > model_.trees.size()) { ntree_limit = static_cast(model_.trees.size()); @@ -525,7 +525,7 @@ class Dart : public GBTree { std::copy(base_margin.begin(), base_margin.end(), out_preds.begin()); } else { std::fill(out_preds.begin(), out_preds.end(), - model_.learner_model_param_->base_score); + model_.learner_model_param->base_score); } const int nthread = omp_get_max_threads(); InitThreadTemp(nthread); @@ -538,18 +538,18 @@ class Dart : public GBTree { DropTrees(false); if (thread_temp_.size() == 0) { thread_temp_.resize(1, RegTree::FVec()); - thread_temp_[0].Init(model_.learner_model_param_->num_feature); + thread_temp_[0].Init(model_.learner_model_param->num_feature); } - out_preds->resize(model_.learner_model_param_->num_output_group); - ntree_limit *= model_.learner_model_param_->num_output_group; + out_preds->resize(model_.learner_model_param->num_output_group); + ntree_limit *= model_.learner_model_param->num_output_group; if (ntree_limit == 0 || ntree_limit > model_.trees.size()) { ntree_limit = static_cast(model_.trees.size()); } // loop over output groups - for (uint32_t gid = 0; gid < model_.learner_model_param_->num_output_group; ++gid) { + for (uint32_t gid = 0; gid < model_.learner_model_param->num_output_group; ++gid) { (*out_preds)[gid] = PredValue(inst, gid, &thread_temp_[0], 0, ntree_limit) + - model_.learner_model_param_->base_score; + model_.learner_model_param->base_score; } } @@ -582,7 +582,7 @@ class Dart : public GBTree { int num_group, unsigned tree_begin, unsigned tree_end) { - CHECK_EQ(num_group, model_.learner_model_param_->num_output_group); + CHECK_EQ(num_group, model_.learner_model_param->num_output_group); std::vector& preds = *out_preds; CHECK_EQ(model_.param.size_leaf_vector, 0) << "size_leaf_vector is enforced to 0 so far"; @@ -635,7 +635,7 @@ class Dart : public GBTree { DMatrix* m, PredictionCacheEntry* predts) override { int num_new_trees = 0; - for (uint32_t gid = 0; gid < model_.learner_model_param_->num_output_group; ++gid) { + for (uint32_t gid = 0; gid < model_.learner_model_param->num_output_group; ++gid) { num_new_trees += new_trees[gid].size(); model_.CommitModel(std::move(new_trees[gid]), gid); } @@ -752,7 +752,7 @@ class Dart : public GBTree { if (prev_thread_temp_size < nthread) { thread_temp_.resize(nthread, RegTree::FVec()); for (int i = prev_thread_temp_size; i < nthread; ++i) { - thread_temp_[i].Init(model_.learner_model_param_->num_feature); + thread_temp_[i].Init(model_.learner_model_param->num_feature); } } } diff --git a/src/gbm/gbtree.h b/src/gbm/gbtree.h index 329d45e2e..6ece5330f 100644 --- a/src/gbm/gbtree.h +++ b/src/gbm/gbtree.h @@ -195,7 +195,7 @@ class GBTree : public GradientBooster { void LoadModel(Json const& in) override; bool AllowLazyCheckPoint() const override { - return model_.learner_model_param_->num_output_group == 1 || + return model_.learner_model_param->num_output_group == 1 || tparam_.updater_seq.find("distcol") != std::string::npos; } @@ -210,7 +210,7 @@ class GBTree : public GradientBooster { unsigned layer_end = 0) const override { CHECK(configured_); // From here on, layer becomes concrete trees. - bst_group_t groups = model_.learner_model_param_->num_output_group; + bst_group_t groups = model_.learner_model_param->num_output_group; uint32_t tree_begin = layer_begin * groups * tparam_.num_parallel_tree; uint32_t tree_end = layer_end * groups * tparam_.num_parallel_tree; if (tree_end == 0 || tree_end > model_.trees.size()) { diff --git a/src/gbm/gbtree_model.cc b/src/gbm/gbtree_model.cc index f2ab0e6fe..a53346797 100644 --- a/src/gbm/gbtree_model.cc +++ b/src/gbm/gbtree_model.cc @@ -40,7 +40,7 @@ void GBTreeModel::Load(dmlc::Stream* fi) { void GBTreeModel::SaveModel(Json* p_out) const { auto& out = *p_out; CHECK_EQ(param.num_trees, static_cast(trees.size())); - out["gbtree_model_param"] = toJson(param); + out["gbtree_model_param"] = ToJson(param); std::vector trees_json; size_t t = 0; for (auto const& tree : trees) { @@ -62,7 +62,7 @@ void GBTreeModel::SaveModel(Json* p_out) const { } void GBTreeModel::LoadModel(Json const& in) { - fromJson(in["gbtree_model_param"], ¶m); + FromJson(in["gbtree_model_param"], ¶m); trees.clear(); trees_to_update.clear(); diff --git a/src/gbm/gbtree_model.h b/src/gbm/gbtree_model.h index a03f163af..7ac7d8f47 100644 --- a/src/gbm/gbtree_model.h +++ b/src/gbm/gbtree_model.h @@ -65,8 +65,8 @@ struct GBTreeModelParam : public dmlc::Parameter { struct GBTreeModel : public Model { public: - explicit GBTreeModel(LearnerModelParam const* learner_model_param) : - learner_model_param_{learner_model_param} {} + explicit GBTreeModel(LearnerModelParam const* learner_model) : + learner_model_param{learner_model} {} void Configure(const Args& cfg) { // initialize model parameters if not yet been initialized. if (trees.size() == 0) { @@ -109,7 +109,7 @@ struct GBTreeModel : public Model { } // base margin - LearnerModelParam const* learner_model_param_; + LearnerModelParam const* learner_model_param; // model parameter GBTreeModelParam param; /*! \brief vector of trees stored in the model */ diff --git a/src/learner.cc b/src/learner.cc index 4f93c4d7b..339f8aa7e 100644 --- a/src/learner.cc +++ b/src/learner.cc @@ -300,7 +300,7 @@ class LearnerConfiguration : public Learner { Version::Load(in, true); auto const& learner_parameters = get(in["learner"]); - fromJson(learner_parameters.at("learner_train_param"), &tparam_); + FromJson(learner_parameters.at("learner_train_param"), &tparam_); auto const& gradient_booster = learner_parameters.at("gradient_booster"); @@ -327,7 +327,7 @@ class LearnerConfiguration : public Learner { Metric::Create(metric_names_[i], &generic_parameters_)); } - fromJson(learner_parameters.at("generic_param"), &generic_parameters_); + FromJson(learner_parameters.at("generic_param"), &generic_parameters_); // make sure the GPU ID is valid in new environment before start running configure. generic_parameters_.ConfigureGpuId(false); @@ -342,7 +342,7 @@ class LearnerConfiguration : public Learner { out["learner"] = Object(); auto& learner_parameters = out["learner"]; - learner_parameters["learner_train_param"] = toJson(tparam_); + learner_parameters["learner_train_param"] = ToJson(tparam_); learner_parameters["learner_model_param"] = mparam_.ToJson(); learner_parameters["gradient_booster"] = Object(); auto& gradient_booster = learner_parameters["gradient_booster"]; @@ -358,7 +358,7 @@ class LearnerConfiguration : public Learner { } learner_parameters["metrics"] = Array(std::move(metrics)); - learner_parameters["generic_param"] = toJson(generic_parameters_); + learner_parameters["generic_param"] = ToJson(generic_parameters_); } void SetParam(const std::string& key, const std::string& value) override { diff --git a/src/linear/coordinate_common.h b/src/linear/coordinate_common.h index 43303f645..45c7a2b4b 100644 --- a/src/linear/coordinate_common.h +++ b/src/linear/coordinate_common.h @@ -252,7 +252,7 @@ class CyclicFeatureSelector : public FeatureSelector { int NextFeature(int iteration, const gbm::GBLinearModel &model, int group_idx, const std::vector &gpair, DMatrix *p_fmat, float alpha, float lambda) override { - return iteration % model.learner_model_param_->num_feature; + return iteration % model.learner_model_param->num_feature; } }; @@ -266,7 +266,7 @@ class ShuffleFeatureSelector : public FeatureSelector { const std::vector &gpair, DMatrix *p_fmat, float alpha, float lambda, int param) override { if (feat_index_.size() == 0) { - feat_index_.resize(model.learner_model_param_->num_feature); + feat_index_.resize(model.learner_model_param->num_feature); std::iota(feat_index_.begin(), feat_index_.end(), 0); } std::shuffle(feat_index_.begin(), feat_index_.end(), common::GlobalRandom()); @@ -275,7 +275,7 @@ class ShuffleFeatureSelector : public FeatureSelector { int NextFeature(int iteration, const gbm::GBLinearModel &model, int group_idx, const std::vector &gpair, DMatrix *p_fmat, float alpha, float lambda) override { - return feat_index_[iteration % model.learner_model_param_->num_feature]; + return feat_index_[iteration % model.learner_model_param->num_feature]; } protected: @@ -291,7 +291,7 @@ class RandomFeatureSelector : public FeatureSelector { int NextFeature(int iteration, const gbm::GBLinearModel &model, int group_idx, const std::vector &gpair, DMatrix *p_fmat, float alpha, float lambda) override { - return common::GlobalRandom()() % model.learner_model_param_->num_feature; + return common::GlobalRandom()() % model.learner_model_param->num_feature; } }; @@ -310,11 +310,11 @@ class GreedyFeatureSelector : public FeatureSelector { const std::vector &gpair, DMatrix *p_fmat, float alpha, float lambda, int param) override { top_k_ = static_cast(param); - const bst_uint ngroup = model.learner_model_param_->num_output_group; + const bst_uint ngroup = model.learner_model_param->num_output_group; if (param <= 0) top_k_ = std::numeric_limits::max(); if (counter_.size() == 0) { counter_.resize(ngroup); - gpair_sums_.resize(model.learner_model_param_->num_feature * ngroup); + gpair_sums_.resize(model.learner_model_param->num_feature * ngroup); } for (bst_uint gid = 0u; gid < ngroup; ++gid) { counter_[gid] = 0u; @@ -327,10 +327,10 @@ class GreedyFeatureSelector : public FeatureSelector { // k-th selected feature for a group auto k = counter_[group_idx]++; // stop after either reaching top-K or going through all the features in a group - if (k >= top_k_ || counter_[group_idx] == model.learner_model_param_->num_feature) return -1; + if (k >= top_k_ || counter_[group_idx] == model.learner_model_param->num_feature) return -1; - const int ngroup = model.learner_model_param_->num_output_group; - const bst_omp_uint nfeat = model.learner_model_param_->num_feature; + const int ngroup = model.learner_model_param->num_output_group; + const bst_omp_uint nfeat = model.learner_model_param->num_feature; // Calculate univariate gradient sums std::fill(gpair_sums_.begin(), gpair_sums_.end(), std::make_pair(0., 0.)); for (const auto &batch : p_fmat->GetBatches()) { @@ -387,8 +387,8 @@ class ThriftyFeatureSelector : public FeatureSelector { DMatrix *p_fmat, float alpha, float lambda, int param) override { top_k_ = static_cast(param); if (param <= 0) top_k_ = std::numeric_limits::max(); - const bst_uint ngroup = model.learner_model_param_->num_output_group; - const bst_omp_uint nfeat = model.learner_model_param_->num_feature; + const bst_uint ngroup = model.learner_model_param->num_output_group; + const bst_omp_uint nfeat = model.learner_model_param->num_feature; if (deltaw_.size() == 0) { deltaw_.resize(nfeat * ngroup); @@ -444,9 +444,9 @@ class ThriftyFeatureSelector : public FeatureSelector { // k-th selected feature for a group auto k = counter_[group_idx]++; // stop after either reaching top-N or going through all the features in a group - if (k >= top_k_ || counter_[group_idx] == model.learner_model_param_->num_feature) return -1; + if (k >= top_k_ || counter_[group_idx] == model.learner_model_param->num_feature) return -1; // note that sorted_idx stores the "long" indices - const size_t grp_offset = group_idx * model.learner_model_param_->num_feature; + const size_t grp_offset = group_idx * model.learner_model_param->num_feature; return static_cast(sorted_idx_[grp_offset + k] - grp_offset); } diff --git a/src/linear/updater_coordinate.cc b/src/linear/updater_coordinate.cc index 74e5d414a..23f227a54 100644 --- a/src/linear/updater_coordinate.cc +++ b/src/linear/updater_coordinate.cc @@ -36,26 +36,26 @@ class CoordinateUpdater : public LinearUpdater { void LoadConfig(Json const& in) override { auto const& config = get(in); - fromJson(config.at("linear_train_param"), &tparam_); - fromJson(config.at("coordinate_param"), &cparam_); + FromJson(config.at("linear_train_param"), &tparam_); + FromJson(config.at("coordinate_param"), &cparam_); } void SaveConfig(Json* p_out) const override { auto& out = *p_out; - out["linear_train_param"] = toJson(tparam_); - out["coordinate_param"] = toJson(cparam_); + out["linear_train_param"] = ToJson(tparam_); + out["coordinate_param"] = ToJson(cparam_); } void Update(HostDeviceVector *in_gpair, DMatrix *p_fmat, gbm::GBLinearModel *model, double sum_instance_weight) override { tparam_.DenormalizePenalties(sum_instance_weight); - const int ngroup = model->learner_model_param_->num_output_group; + const int ngroup = model->learner_model_param->num_output_group; // update bias for (int group_idx = 0; group_idx < ngroup; ++group_idx) { auto grad = GetBiasGradientParallel(group_idx, ngroup, in_gpair->ConstHostVector(), p_fmat); auto dbias = static_cast(tparam_.learning_rate * CoordinateDeltaBias(grad.first, grad.second)); - model->bias()[group_idx] += dbias; + model->Bias()[group_idx] += dbias; UpdateBiasResidualParallel(group_idx, ngroup, dbias, &in_gpair->HostVector(), p_fmat); } @@ -65,7 +65,7 @@ class CoordinateUpdater : public LinearUpdater { tparam_.reg_lambda_denorm, cparam_.top_k); // update weights for (int group_idx = 0; group_idx < ngroup; ++group_idx) { - for (unsigned i = 0U; i < model->learner_model_param_->num_feature; i++) { + for (unsigned i = 0U; i < model->learner_model_param->num_feature; i++) { int fidx = selector_->NextFeature (i, *model, group_idx, in_gpair->ConstHostVector(), p_fmat, tparam_.reg_alpha_denorm, tparam_.reg_lambda_denorm); @@ -78,7 +78,7 @@ class CoordinateUpdater : public LinearUpdater { inline void UpdateFeature(int fidx, int group_idx, std::vector *in_gpair, DMatrix *p_fmat, gbm::GBLinearModel *model) { - const int ngroup = model->learner_model_param_->num_output_group; + const int ngroup = model->learner_model_param->num_output_group; bst_float &w = (*model)[fidx][group_idx]; auto gradient = GetGradientParallel(group_idx, ngroup, fidx, *in_gpair, p_fmat); diff --git a/src/linear/updater_gpu_coordinate.cu b/src/linear/updater_gpu_coordinate.cu index edf207a9c..652207516 100644 --- a/src/linear/updater_gpu_coordinate.cu +++ b/src/linear/updater_gpu_coordinate.cu @@ -44,13 +44,13 @@ class GPUCoordinateUpdater : public LinearUpdater { // NOLINT void LoadConfig(Json const& in) override { auto const& config = get(in); - fromJson(config.at("linear_train_param"), &tparam_); - fromJson(config.at("coordinate_param"), &coord_param_); + FromJson(config.at("linear_train_param"), &tparam_); + FromJson(config.at("coordinate_param"), &coord_param_); } void SaveConfig(Json* p_out) const override { auto& out = *p_out; - out["linear_train_param"] = toJson(tparam_); - out["coordinate_param"] = toJson(coord_param_); + out["linear_train_param"] = ToJson(tparam_); + out["coordinate_param"] = ToJson(coord_param_); } void LazyInitDevice(DMatrix *p_fmat, const LearnerModelParam &model_param) { @@ -103,7 +103,7 @@ class GPUCoordinateUpdater : public LinearUpdater { // NOLINT gbm::GBLinearModel *model, double sum_instance_weight) override { tparam_.DenormalizePenalties(sum_instance_weight); monitor_.Start("LazyInitDevice"); - this->LazyInitDevice(p_fmat, *(model->learner_model_param_)); + this->LazyInitDevice(p_fmat, *(model->learner_model_param)); monitor_.Stop("LazyInitDevice"); monitor_.Start("UpdateGpair"); @@ -122,9 +122,9 @@ class GPUCoordinateUpdater : public LinearUpdater { // NOLINT tparam_.reg_alpha_denorm, tparam_.reg_lambda_denorm, coord_param_.top_k); monitor_.Start("UpdateFeature"); - for (auto group_idx = 0; group_idx < model->learner_model_param_->num_output_group; + for (auto group_idx = 0; group_idx < model->learner_model_param->num_output_group; ++group_idx) { - for (auto i = 0U; i < model->learner_model_param_->num_feature; i++) { + for (auto i = 0U; i < model->learner_model_param->num_feature; i++) { auto fidx = selector_->NextFeature( i, *model, group_idx, in_gpair->ConstHostVector(), p_fmat, tparam_.reg_alpha_denorm, tparam_.reg_lambda_denorm); @@ -136,21 +136,21 @@ class GPUCoordinateUpdater : public LinearUpdater { // NOLINT } void UpdateBias(DMatrix *p_fmat, gbm::GBLinearModel *model) { - for (int group_idx = 0; group_idx < model->learner_model_param_->num_output_group; + for (int group_idx = 0; group_idx < model->learner_model_param->num_output_group; ++group_idx) { // Get gradient auto grad = GradientPair(0, 0); if (learner_param_->gpu_id >= 0) { - grad = GetBiasGradient(group_idx, model->learner_model_param_->num_output_group); + grad = GetBiasGradient(group_idx, model->learner_model_param->num_output_group); } auto dbias = static_cast( tparam_.learning_rate * CoordinateDeltaBias(grad.GetGrad(), grad.GetHess())); - model->bias()[group_idx] += dbias; + model->Bias()[group_idx] += dbias; // Update residual if (learner_param_->gpu_id >= 0) { - UpdateBiasResidual(dbias, group_idx, model->learner_model_param_->num_output_group); + UpdateBiasResidual(dbias, group_idx, model->learner_model_param->num_output_group); } } } @@ -162,7 +162,7 @@ class GPUCoordinateUpdater : public LinearUpdater { // NOLINT // Get gradient auto grad = GradientPair(0, 0); if (learner_param_->gpu_id >= 0) { - grad = GetGradient(group_idx, model->learner_model_param_->num_output_group, fidx); + grad = GetGradient(group_idx, model->learner_model_param->num_output_group, fidx); } auto dw = static_cast(tparam_.learning_rate * CoordinateDelta(grad.GetGrad(), grad.GetHess(), @@ -171,7 +171,7 @@ class GPUCoordinateUpdater : public LinearUpdater { // NOLINT w += dw; if (learner_param_->gpu_id >= 0) { - UpdateResidual(dw, group_idx, model->learner_model_param_->num_output_group, fidx); + UpdateResidual(dw, group_idx, model->learner_model_param->num_output_group, fidx); } } @@ -186,7 +186,7 @@ class GPUCoordinateUpdater : public LinearUpdater { // NOLINT counting, f); auto perm = thrust::make_permutation_iterator(gpair_.data(), skip); - return dh::SumReduction(temp_, perm, num_row_); + return dh::SumReduction(&temp_, perm, num_row_); } // This needs to be public because of the __device__ lambda. @@ -214,7 +214,7 @@ class GPUCoordinateUpdater : public LinearUpdater { // NOLINT }; // NOLINT thrust::transform_iterator multiply_iterator(counting, f); - return dh::SumReduction(temp_, multiply_iterator, col_size); + return dh::SumReduction(&temp_, multiply_iterator, col_size); } // This needs to be public because of the __device__ lambda. diff --git a/src/linear/updater_shotgun.cc b/src/linear/updater_shotgun.cc index 10cb7912b..08604ce73 100644 --- a/src/linear/updater_shotgun.cc +++ b/src/linear/updater_shotgun.cc @@ -25,18 +25,18 @@ class ShotgunUpdater : public LinearUpdater { } void LoadConfig(Json const& in) override { auto const& config = get(in); - fromJson(config.at("linear_train_param"), ¶m_); + FromJson(config.at("linear_train_param"), ¶m_); } void SaveConfig(Json* p_out) const override { auto& out = *p_out; - out["linear_train_param"] = toJson(param_); + out["linear_train_param"] = ToJson(param_); } void Update(HostDeviceVector *in_gpair, DMatrix *p_fmat, gbm::GBLinearModel *model, double sum_instance_weight) override { auto &gpair = in_gpair->HostVector(); param_.DenormalizePenalties(sum_instance_weight); - const int ngroup = model->learner_model_param_->num_output_group; + const int ngroup = model->learner_model_param->num_output_group; // update bias for (int gid = 0; gid < ngroup; ++gid) { @@ -44,7 +44,7 @@ class ShotgunUpdater : public LinearUpdater { in_gpair->ConstHostVector(), p_fmat); auto dbias = static_cast(param_.learning_rate * CoordinateDeltaBias(grad.first, grad.second)); - model->bias()[gid] += dbias; + model->Bias()[gid] += dbias; UpdateBiasResidualParallel(gid, ngroup, dbias, &in_gpair->HostVector(), p_fmat); } diff --git a/src/metric/elementwise_metric.cu b/src/metric/elementwise_metric.cu index 4d26ceba2..72381d5d6 100644 --- a/src/metric/elementwise_metric.cu +++ b/src/metric/elementwise_metric.cu @@ -319,7 +319,7 @@ struct EvalTweedieNLogLik { */ template struct EvalEWiseBase : public Metric { - EvalEWiseBase() : policy_{}, reducer_{policy_} {} + EvalEWiseBase() = default; explicit EvalEWiseBase(char const* policy_param) : policy_{policy_param}, reducer_{policy_} {} @@ -351,8 +351,7 @@ struct EvalEWiseBase : public Metric { private: Policy policy_; - - ElementWiseMetricsReduction reducer_; + ElementWiseMetricsReduction reducer_{policy_}; }; XGBOOST_REGISTER_METRIC(RMSE, "rmse") diff --git a/src/metric/metric_common.h b/src/metric/metric_common.h index 1549edf1e..d676797f4 100644 --- a/src/metric/metric_common.h +++ b/src/metric/metric_common.h @@ -62,11 +62,11 @@ struct EvalRankConfig { }; class PackedReduceResult { - double residue_sum_; - double weights_sum_; + double residue_sum_ { 0 }; + double weights_sum_ { 0 }; public: - XGBOOST_DEVICE PackedReduceResult() : residue_sum_{0}, weights_sum_{0} {} + XGBOOST_DEVICE PackedReduceResult() {} // NOLINT XGBOOST_DEVICE PackedReduceResult(double residue, double weight) : residue_sum_{residue}, weights_sum_{weight} {} diff --git a/src/metric/rank_metric.cu b/src/metric/rank_metric.cu index e19936b57..290b1fb02 100644 --- a/src/metric/rank_metric.cu +++ b/src/metric/rank_metric.cu @@ -135,7 +135,6 @@ struct EvalNDCGGpu { dh::caching_device_vector &dcgs(*dcgptr); // Group info on device const auto &dgroups = pred_sorter.GetGroupsSpan(); - const auto ngroups = pred_sorter.GetNumGroups(); const auto &dgroup_idx = pred_sorter.GetGroupSegmentsSpan(); // First, determine non zero labels in the dataset individually diff --git a/src/metric/survival_metric.cc b/src/metric/survival_metric.cc index 252ee40cd..58518f007 100644 --- a/src/metric/survival_metric.cc +++ b/src/metric/survival_metric.cc @@ -40,11 +40,11 @@ struct EvalAFT : public Metric { void SaveConfig(Json* p_out) const override { auto& out = *p_out; out["name"] = String(this->Name()); - out["aft_loss_param"] = toJson(param_); + out["aft_loss_param"] = ToJson(param_); } void LoadConfig(Json const& in) override { - fromJson(in["aft_loss_param"], ¶m_); + FromJson(in["aft_loss_param"], ¶m_); } bst_float Eval(const HostDeviceVector &preds, diff --git a/src/objective/aft_obj.cc b/src/objective/aft_obj.cc index 1935d8b71..0a5df1391 100644 --- a/src/objective/aft_obj.cc +++ b/src/objective/aft_obj.cc @@ -95,11 +95,11 @@ class AFTObj : public ObjFunction { void SaveConfig(Json* p_out) const override { auto& out = *p_out; out["name"] = String("survival:aft"); - out["aft_loss_param"] = toJson(param_); + out["aft_loss_param"] = ToJson(param_); } void LoadConfig(Json const& in) override { - fromJson(in["aft_loss_param"], ¶m_); + FromJson(in["aft_loss_param"], ¶m_); loss_.reset(new AFTLoss(param_.aft_loss_distribution)); } diff --git a/src/objective/multiclass_obj.cu b/src/objective/multiclass_obj.cu index dce195629..29af5e0d2 100644 --- a/src/objective/multiclass_obj.cu +++ b/src/objective/multiclass_obj.cu @@ -170,11 +170,11 @@ class SoftmaxMultiClassObj : public ObjFunction { } else { out["name"] = String("multi:softmax"); } - out["softmax_multiclass_param"] = toJson(param_); + out["softmax_multiclass_param"] = ToJson(param_); } void LoadConfig(Json const& in) override { - fromJson(in["softmax_multiclass_param"], ¶m_); + FromJson(in["softmax_multiclass_param"], ¶m_); } private: diff --git a/src/objective/rank_obj.cu b/src/objective/rank_obj.cu index a4f3dad4f..5d92d3b54 100644 --- a/src/objective/rank_obj.cu +++ b/src/objective/rank_obj.cu @@ -552,16 +552,15 @@ class MAPLambdaWeightComputer const auto *dhits_arr = dhits.data().get(); // Group info on device const auto &dgroups = segment_label_sorter.GetGroupsSpan(); - uint32_t ngroups = segment_label_sorter.GetNumGroups(); auto ComputeItemPrecisionLambda = [=] __device__(uint32_t idx) { if (unsorted_labels[pred_original_pos[idx]] > 0.0f) { auto idx_within_group = (idx - dgroups[group_segments[idx]]) + 1; - return MAPStats(static_cast(dhits_arr[idx]) / idx_within_group, + return MAPStats{static_cast(dhits_arr[idx]) / idx_within_group, static_cast(dhits_arr[idx] - 1) / idx_within_group, static_cast(dhits_arr[idx] + 1) / idx_within_group, - 1.0f); + 1.0f}; } - return MAPStats(); + return MAPStats{}; }; // NOLINT thrust::transform(thrust::make_counting_iterator(static_cast(0)), @@ -784,14 +783,11 @@ class LambdaRankObj : public ObjFunction { void SaveConfig(Json* p_out) const override { auto& out = *p_out; out["name"] = String(LambdaWeightComputerT::Name()); - out["lambda_rank_param"] = Object(); - for (auto const& kv : param_.__DICT__()) { - out["lambda_rank_param"][kv.first] = kv.second; - } + out["lambda_rank_param"] = ToJson(param_); } void LoadConfig(Json const& in) override { - fromJson(in["lambda_rank_param"], ¶m_); + FromJson(in["lambda_rank_param"], ¶m_); } private: diff --git a/src/objective/regression_obj.cu b/src/objective/regression_obj.cu index 90799d226..414f9d2aa 100644 --- a/src/objective/regression_obj.cu +++ b/src/objective/regression_obj.cu @@ -126,11 +126,11 @@ class RegLossObj : public ObjFunction { void SaveConfig(Json* p_out) const override { auto& out = *p_out; out["name"] = String(Loss::Name()); - out["reg_loss_param"] = toJson(param_); + out["reg_loss_param"] = ToJson(param_); } void LoadConfig(Json const& in) override { - fromJson(in["reg_loss_param"], ¶m_); + FromJson(in["reg_loss_param"], ¶m_); } protected: @@ -253,11 +253,11 @@ class PoissonRegression : public ObjFunction { void SaveConfig(Json* p_out) const override { auto& out = *p_out; out["name"] = String("count:poisson"); - out["poisson_regression_param"] = toJson(param_); + out["poisson_regression_param"] = ToJson(param_); } void LoadConfig(Json const& in) override { - fromJson(in["poisson_regression_param"], ¶m_); + FromJson(in["poisson_regression_param"], ¶m_); } private: @@ -546,10 +546,10 @@ class TweedieRegression : public ObjFunction { void SaveConfig(Json* p_out) const override { auto& out = *p_out; out["name"] = String("reg:tweedie"); - out["tweedie_regression_param"] = toJson(param_); + out["tweedie_regression_param"] = ToJson(param_); } void LoadConfig(Json const& in) override { - fromJson(in["tweedie_regression_param"], ¶m_); + FromJson(in["tweedie_regression_param"], ¶m_); } private: diff --git a/src/predictor/cpu_predictor.cc b/src/predictor/cpu_predictor.cc index 2fc15731d..ebc15128b 100644 --- a/src/predictor/cpu_predictor.cc +++ b/src/predictor/cpu_predictor.cc @@ -107,7 +107,7 @@ void PredictBatchKernel(DataView batch, std::vector *out_preds, int32_t tree_end, std::vector *p_thread_temp) { auto& thread_temp = *p_thread_temp; - int32_t const num_group = model.learner_model_param_->num_output_group; + int32_t const num_group = model.learner_model_param->num_output_group; std::vector &preds = *out_preds; CHECK_EQ(model.param.size_leaf_vector, 0) @@ -168,10 +168,10 @@ class CPUPredictor : public Predictor { int32_t tree_end) { std::lock_guard guard(lock_); const int threads = omp_get_max_threads(); - InitThreadTemp(threads, model.learner_model_param_->num_feature, &this->thread_temp_); + InitThreadTemp(threads, model.learner_model_param->num_feature, &this->thread_temp_); for (auto const& batch : p_fmat->GetBatches()) { CHECK_EQ(out_preds->size(), - p_fmat->Info().num_row_ * model.learner_model_param_->num_output_group); + p_fmat->Info().num_row_ * model.learner_model_param->num_output_group); size_t constexpr kUnroll = 8; PredictBatchKernel(SparsePageView{&batch}, out_preds, model, tree_begin, tree_end, &thread_temp_); @@ -181,8 +181,8 @@ class CPUPredictor : public Predictor { void InitOutPredictions(const MetaInfo& info, HostDeviceVector* out_preds, const gbm::GBTreeModel& model) const { - CHECK_NE(model.learner_model_param_->num_output_group, 0); - size_t n = model.learner_model_param_->num_output_group * info.num_row_; + CHECK_NE(model.learner_model_param->num_output_group, 0); + size_t n = model.learner_model_param->num_output_group * info.num_row_; const auto& base_margin = info.base_margin_.HostVector(); out_preds->Resize(n); std::vector& out_preds_h = out_preds->HostVector(); @@ -194,19 +194,19 @@ class CPUPredictor : public Predictor { std::ostringstream oss; oss << "Ignoring the base margin, since it has incorrect length. " << "The base margin must be an array of length "; - if (model.learner_model_param_->num_output_group > 1) { + if (model.learner_model_param->num_output_group > 1) { oss << "[num_class] * [number of data points], i.e. " - << model.learner_model_param_->num_output_group << " * " << info.num_row_ + << model.learner_model_param->num_output_group << " * " << info.num_row_ << " = " << n << ". "; } else { oss << "[number of data points], i.e. " << info.num_row_ << ". "; } oss << "Instead, all data points will use " - << "base_score = " << model.learner_model_param_->base_score; + << "base_score = " << model.learner_model_param->base_score; LOG(WARNING) << oss.str(); } std::fill(out_preds_h.begin(), out_preds_h.end(), - model.learner_model_param_->base_score); + model.learner_model_param->base_score); } } @@ -231,7 +231,7 @@ class CPUPredictor : public Predictor { this->InitOutPredictions(dmat->Info(), out_preds, model); } - uint32_t const output_groups = model.learner_model_param_->num_output_group; + uint32_t const output_groups = model.learner_model_param->num_output_group; CHECK_NE(output_groups, 0); // Right now we just assume ntree_limit provided by users means number of tree layers // in the context of multi-output model @@ -272,7 +272,7 @@ class CPUPredictor : public Predictor { uint32_t tree_begin, uint32_t tree_end) const { auto threads = omp_get_max_threads(); auto m = dmlc::get(x); - CHECK_EQ(m.NumColumns(), model.learner_model_param_->num_feature) + CHECK_EQ(m.NumColumns(), model.learner_model_param->num_feature) << "Number of columns in data must equal to trained model."; MetaInfo info; info.num_col_ = m.NumColumns(); @@ -281,7 +281,7 @@ class CPUPredictor : public Predictor { std::vector workspace(info.num_col_ * 8 * threads); auto &predictions = out_preds->predictions.HostVector(); std::vector thread_temp; - InitThreadTemp(threads, model.learner_model_param_->num_feature, &thread_temp); + InitThreadTemp(threads, model.learner_model_param->num_feature, &thread_temp); size_t constexpr kUnroll = 8; PredictBatchKernel(AdapterView( &m, missing, common::Span{workspace}), @@ -307,29 +307,29 @@ class CPUPredictor : public Predictor { const gbm::GBTreeModel& model, unsigned ntree_limit) override { if (thread_temp_.size() == 0) { thread_temp_.resize(1, RegTree::FVec()); - thread_temp_[0].Init(model.learner_model_param_->num_feature); + thread_temp_[0].Init(model.learner_model_param->num_feature); } - ntree_limit *= model.learner_model_param_->num_output_group; + ntree_limit *= model.learner_model_param->num_output_group; if (ntree_limit == 0 || ntree_limit > model.trees.size()) { ntree_limit = static_cast(model.trees.size()); } - out_preds->resize(model.learner_model_param_->num_output_group * + out_preds->resize(model.learner_model_param->num_output_group * (model.param.size_leaf_vector + 1)); // loop over output groups - for (uint32_t gid = 0; gid < model.learner_model_param_->num_output_group; ++gid) { + for (uint32_t gid = 0; gid < model.learner_model_param->num_output_group; ++gid) { (*out_preds)[gid] = PredValue(inst, model.trees, model.tree_info, gid, &thread_temp_[0], 0, ntree_limit) + - model.learner_model_param_->base_score; + model.learner_model_param->base_score; } } void PredictLeaf(DMatrix* p_fmat, std::vector* out_preds, const gbm::GBTreeModel& model, unsigned ntree_limit) override { const int nthread = omp_get_max_threads(); - InitThreadTemp(nthread, model.learner_model_param_->num_feature, &this->thread_temp_); + InitThreadTemp(nthread, model.learner_model_param->num_feature, &this->thread_temp_); const MetaInfo& info = p_fmat->Info(); // number of valid trees - ntree_limit *= model.learner_model_param_->num_output_group; + ntree_limit *= model.learner_model_param->num_output_group; if (ntree_limit == 0 || ntree_limit > model.trees.size()) { ntree_limit = static_cast(model.trees.size()); } @@ -360,20 +360,20 @@ class CPUPredictor : public Predictor { bool approximate, int condition, unsigned condition_feature) override { const int nthread = omp_get_max_threads(); - InitThreadTemp(nthread, model.learner_model_param_->num_feature, &this->thread_temp_); + InitThreadTemp(nthread, model.learner_model_param->num_feature, &this->thread_temp_); const MetaInfo& info = p_fmat->Info(); // number of valid trees - ntree_limit *= model.learner_model_param_->num_output_group; + ntree_limit *= model.learner_model_param->num_output_group; if (ntree_limit == 0 || ntree_limit > model.trees.size()) { ntree_limit = static_cast(model.trees.size()); } - const int ngroup = model.learner_model_param_->num_output_group; + const int ngroup = model.learner_model_param->num_output_group; CHECK_NE(ngroup, 0); - size_t const ncolumns = model.learner_model_param_->num_feature + 1; + size_t const ncolumns = model.learner_model_param->num_feature + 1; CHECK_NE(ncolumns, 0); // allocate space for (number of features + bias) times the number of rows std::vector& contribs = *out_contribs; - contribs.resize(info.num_row_ * ncolumns * model.learner_model_param_->num_output_group); + contribs.resize(info.num_row_ * ncolumns * model.learner_model_param->num_output_group); // make sure contributions is zeroed, we could be reusing a previously // allocated one std::fill(contribs.begin(), contribs.end(), 0); @@ -418,7 +418,7 @@ class CPUPredictor : public Predictor { if (base_margin.size() != 0) { p_contribs[ncolumns - 1] += base_margin[row_idx * ngroup + gid]; } else { - p_contribs[ncolumns - 1] += model.learner_model_param_->base_score; + p_contribs[ncolumns - 1] += model.learner_model_param->base_score; } } } @@ -430,8 +430,8 @@ class CPUPredictor : public Predictor { std::vector* tree_weights, bool approximate) override { const MetaInfo& info = p_fmat->Info(); - const int ngroup = model.learner_model_param_->num_output_group; - size_t const ncolumns = model.learner_model_param_->num_feature; + const int ngroup = model.learner_model_param->num_output_group; + size_t const ncolumns = model.learner_model_param->num_feature; const unsigned row_chunk = ngroup * (ncolumns + 1) * (ncolumns + 1); const unsigned mrow_chunk = (ncolumns + 1) * (ncolumns + 1); const unsigned crow_chunk = ngroup * (ncolumns + 1); diff --git a/src/predictor/gpu_predictor.cu b/src/predictor/gpu_predictor.cu index a77e564c9..9e6043ed5 100644 --- a/src/predictor/gpu_predictor.cu +++ b/src/predictor/gpu_predictor.cu @@ -267,7 +267,7 @@ class DeviceModel { cudaMemcpyHostToDevice)); this->tree_beg_ = tree_begin; this->tree_end_ = tree_end; - this->num_group = model.learner_model_param_->num_output_group; + this->num_group = model.learner_model_param->num_output_group; } void Init(const gbm::GBTreeModel& model, size_t tree_begin, size_t tree_end, int32_t gpu_id) { @@ -359,9 +359,9 @@ class GPUPredictor : public xgboost::Predictor { } else { size_t batch_offset = 0; for (auto &batch : dmat->GetBatches()) { - this->PredictInternal(batch, model.learner_model_param_->num_feature, + this->PredictInternal(batch, model.learner_model_param->num_feature, out_preds, batch_offset); - batch_offset += batch.Size() * model.learner_model_param_->num_output_group; + batch_offset += batch.Size() * model.learner_model_param->num_output_group; } } } @@ -396,7 +396,7 @@ class GPUPredictor : public xgboost::Predictor { this->InitOutPredictions(dmat->Info(), out_preds, model); } - uint32_t const output_groups = model.learner_model_param_->num_output_group; + uint32_t const output_groups = model.learner_model_param->num_output_group; CHECK_NE(output_groups, 0); uint32_t real_ntree_limit = ntree_limit * output_groups; @@ -434,12 +434,12 @@ class GPUPredictor : public xgboost::Predictor { PredictionCacheEntry *out_preds, uint32_t tree_begin, uint32_t tree_end) const { auto max_shared_memory_bytes = dh::MaxSharedMemory(this->generic_param_->gpu_id); - uint32_t const output_groups = model.learner_model_param_->num_output_group; + uint32_t const output_groups = model.learner_model_param->num_output_group; DeviceModel d_model; d_model.Init(model, tree_begin, tree_end, this->generic_param_->gpu_id); auto m = dmlc::get(x); - CHECK_EQ(m.NumColumns(), model.learner_model_param_->num_feature) + CHECK_EQ(m.NumColumns(), model.learner_model_param->num_feature) << "Number of columns in data must equal to trained model."; CHECK_EQ(this->generic_param_->gpu_id, m.DeviceIdx()) << "XGBoost is running on device: " << this->generic_param_->gpu_id << ", " @@ -473,7 +473,6 @@ class GPUPredictor : public xgboost::Predictor { void InplacePredict(dmlc::any const &x, const gbm::GBTreeModel &model, float missing, PredictionCacheEntry *out_preds, uint32_t tree_begin, unsigned tree_end) const override { - auto max_shared_memory_bytes = dh::MaxSharedMemory(this->generic_param_->gpu_id); if (x.type() == typeid(data::CupyAdapter)) { this->DispatchedInplacePredict( x, model, missing, out_preds, tree_begin, tree_end); @@ -489,7 +488,7 @@ class GPUPredictor : public xgboost::Predictor { void InitOutPredictions(const MetaInfo& info, HostDeviceVector* out_preds, const gbm::GBTreeModel& model) const { - size_t n_classes = model.learner_model_param_->num_output_group; + size_t n_classes = model.learner_model_param->num_output_group; size_t n = n_classes * info.num_row_; const HostDeviceVector& base_margin = info.base_margin_; out_preds->SetDevice(generic_param_->gpu_id); @@ -498,7 +497,7 @@ class GPUPredictor : public xgboost::Predictor { CHECK_EQ(base_margin.Size(), n); out_preds->Copy(base_margin); } else { - out_preds->Fill(model.learner_model_param_->base_score); + out_preds->Fill(model.learner_model_param->base_score); } } diff --git a/src/tree/constraints.cu b/src/tree/constraints.cu index a8d726814..857dcfd55 100644 --- a/src/tree/constraints.cu +++ b/src/tree/constraints.cu @@ -128,7 +128,7 @@ void FeatureInteractionConstraint::Configure( s_sets_ptr_ = dh::ToSpan(d_sets_ptr_); d_feature_buffer_storage_.resize(LBitField64::ComputeStorageSize(n_features)); - feature_buffer_ = dh::ToSpan(d_feature_buffer_storage_); + feature_buffer_ = LBitField64{dh::ToSpan(d_feature_buffer_storage_)}; // --- Initialize result buffers. output_buffer_bits_storage_.resize(LBitField64::ComputeStorageSize(n_features)); diff --git a/src/tree/constraints.h b/src/tree/constraints.h index 222d763b0..facc5ebee 100644 --- a/src/tree/constraints.h +++ b/src/tree/constraints.h @@ -31,8 +31,6 @@ class FeatureInteractionConstraintHost { // splits_[nid] contains the set of all feature IDs that have been used for // splits in node nid and its parents std::vector< std::unordered_set > splits_; - - std::vector return_buffer; // string passed by user. std::string interaction_constraint_str_; // number of features in DMatrix/Booster diff --git a/src/tree/gpu_hist/gradient_based_sampler.cu b/src/tree/gpu_hist/gradient_based_sampler.cu index 888e39931..169eee059 100644 --- a/src/tree/gpu_hist/gradient_based_sampler.cu +++ b/src/tree/gpu_hist/gradient_based_sampler.cu @@ -153,7 +153,7 @@ ExternalMemoryNoSampling::ExternalMemoryNoSampling(EllpackPageImpl* page, size_t n_rows, const BatchParam& batch_param) : batch_param_(batch_param), - page_(new EllpackPageImpl(batch_param.gpu_id, page->cuts_, page->is_dense, + page_(new EllpackPageImpl(batch_param.gpu_id, page->Cuts(), page->is_dense, page->row_stride, n_rows)) {} GradientBasedSample ExternalMemoryNoSampling::Sample(common::Span gpair, @@ -201,7 +201,6 @@ GradientBasedSample ExternalMemoryUniformSampling::Sample(common::SpanInfo().num_row_; // Compact gradient pairs. gpair_.resize(sample_rows); @@ -219,7 +218,7 @@ GradientBasedSample ExternalMemoryUniformSampling::Sample(common::Spancuts_, original_page_->is_dense, + batch_param_.gpu_id, original_page_->Cuts(), original_page_->is_dense, original_page_->row_stride, sample_rows)); // Compact the ELLPACK pages into the single sample page. @@ -299,7 +298,7 @@ GradientBasedSample ExternalMemoryGradientBasedSampling::Sample(common::Spancuts_, + page_.reset(new EllpackPageImpl(batch_param_.gpu_id, original_page_->Cuts(), original_page_->is_dense, original_page_->row_stride, sample_rows)); diff --git a/src/tree/gpu_hist/row_partitioner.cu b/src/tree/gpu_hist/row_partitioner.cu index 176740f12..7427362e9 100644 --- a/src/tree/gpu_hist/row_partitioner.cu +++ b/src/tree/gpu_hist/row_partitioner.cu @@ -64,54 +64,55 @@ void RowPartitioner::SortPosition(common::Span position, cub::DeviceScan::ExclusiveSum(temp_storage.data().get(), temp_storage_bytes, in_itr, out_itr, position.size(), stream); } + RowPartitioner::RowPartitioner(int device_idx, size_t num_rows) - : device_idx(device_idx) { - dh::safe_cuda(cudaSetDevice(device_idx)); - ridx_a.resize(num_rows); - ridx_b.resize(num_rows); - position_a.resize(num_rows); - position_b.resize(num_rows); - ridx = dh::DoubleBuffer{&ridx_a, &ridx_b}; - position = dh::DoubleBuffer{&position_a, &position_b}; - ridx_segments.emplace_back(Segment(0, num_rows)); + : device_idx_(device_idx) { + dh::safe_cuda(cudaSetDevice(device_idx_)); + ridx_a_.resize(num_rows); + ridx_b_.resize(num_rows); + position_a_.resize(num_rows); + position_b_.resize(num_rows); + ridx_ = dh::DoubleBuffer{&ridx_a_, &ridx_b_}; + position_ = dh::DoubleBuffer{&position_a_, &position_b_}; + ridx_segments_.emplace_back(Segment(0, num_rows)); thrust::sequence( - thrust::device_pointer_cast(ridx.CurrentSpan().data()), - thrust::device_pointer_cast(ridx.CurrentSpan().data() + ridx.Size())); + thrust::device_pointer_cast(ridx_.CurrentSpan().data()), + thrust::device_pointer_cast(ridx_.CurrentSpan().data() + ridx_.Size())); thrust::fill( - thrust::device_pointer_cast(position.Current()), - thrust::device_pointer_cast(position.Current() + position.Size()), 0); - left_counts.resize(256); - thrust::fill(left_counts.begin(), left_counts.end(), 0); - streams.resize(2); - for (auto& stream : streams) { + thrust::device_pointer_cast(position_.Current()), + thrust::device_pointer_cast(position_.Current() + position_.Size()), 0); + left_counts_.resize(256); + thrust::fill(left_counts_.begin(), left_counts_.end(), 0); + streams_.resize(2); + for (auto& stream : streams_) { dh::safe_cuda(cudaStreamCreate(&stream)); } } RowPartitioner::~RowPartitioner() { - dh::safe_cuda(cudaSetDevice(device_idx)); - for (auto& stream : streams) { + dh::safe_cuda(cudaSetDevice(device_idx_)); + for (auto& stream : streams_) { dh::safe_cuda(cudaStreamDestroy(stream)); } } common::Span RowPartitioner::GetRows( bst_node_t nidx) { - auto segment = ridx_segments.at(nidx); + auto segment = ridx_segments_.at(nidx); // Return empty span here as a valid result // Will error if we try to construct a span from a pointer with size 0 if (segment.Size() == 0) { return common::Span(); } - return ridx.CurrentSpan().subspan(segment.begin, segment.Size()); + return ridx_.CurrentSpan().subspan(segment.begin, segment.Size()); } common::Span RowPartitioner::GetRows() { - return ridx.CurrentSpan(); + return ridx_.CurrentSpan(); } common::Span RowPartitioner::GetPosition() { - return position.CurrentSpan(); + return position_.CurrentSpan(); } std::vector RowPartitioner::GetRowsHost( bst_node_t nidx) { @@ -135,22 +136,22 @@ void RowPartitioner::SortPositionAndCopy(const Segment& segment, cudaStream_t stream) { SortPosition( // position_in - common::Span(position.Current() + segment.begin, + common::Span(position_.Current() + segment.begin, segment.Size()), // position_out - common::Span(position.other() + segment.begin, - segment.Size()), + common::Span(position_.Other() + segment.begin, + segment.Size()), // row index in - common::Span(ridx.Current() + segment.begin, segment.Size()), + common::Span(ridx_.Current() + segment.begin, segment.Size()), // row index out - common::Span(ridx.other() + segment.begin, segment.Size()), + common::Span(ridx_.Other() + segment.begin, segment.Size()), left_nidx, right_nidx, d_left_count, stream); // Copy back key/value - const auto d_position_current = position.Current() + segment.begin; - const auto d_position_other = position.other() + segment.begin; - const auto d_ridx_current = ridx.Current() + segment.begin; - const auto d_ridx_other = ridx.other() + segment.begin; - dh::LaunchN(device_idx, segment.Size(), stream, [=] __device__(size_t idx) { + const auto d_position_current = position_.Current() + segment.begin; + const auto d_position_other = position_.Other() + segment.begin; + const auto d_ridx_current = ridx_.Current() + segment.begin; + const auto d_ridx_other = ridx_.Other() + segment.begin; + dh::LaunchN(device_idx_, segment.Size(), stream, [=] __device__(size_t idx) { d_position_current[idx] = d_position_other[idx]; d_ridx_current[idx] = d_ridx_other[idx]; }); diff --git a/src/tree/gpu_hist/row_partitioner.cuh b/src/tree/gpu_hist/row_partitioner.cuh index 4818d71ab..03334efe6 100644 --- a/src/tree/gpu_hist/row_partitioner.cuh +++ b/src/tree/gpu_hist/row_partitioner.cuh @@ -36,7 +36,7 @@ class RowPartitioner { static constexpr bst_node_t kIgnoredTreePosition = -1; private: - int device_idx; + int device_idx_; /*! \brief In here if you want to find the rows belong to a node nid, first you need to * get the indices segment from ridx_segments[nid], then get the row index that * represents position of row in input data X. `RowPartitioner::GetRows` would be a @@ -45,22 +45,22 @@ class RowPartitioner { * node id -> segment -> indices of rows belonging to node */ /*! \brief Range of row index for each node, pointers into ridx below. */ - std::vector ridx_segments; - dh::caching_device_vector ridx_a; - dh::caching_device_vector ridx_b; - dh::caching_device_vector position_a; - dh::caching_device_vector position_b; + std::vector ridx_segments_; + dh::caching_device_vector ridx_a_; + dh::caching_device_vector ridx_b_; + dh::caching_device_vector position_a_; + dh::caching_device_vector position_b_; /*! \brief mapping for node id -> rows. * This looks like: * node id | 1 | 2 | * rows idx | 3, 5, 1 | 13, 31 | */ - dh::DoubleBuffer ridx; + dh::DoubleBuffer ridx_; /*! \brief mapping for row -> node id. */ - dh::DoubleBuffer position; + dh::DoubleBuffer position_; dh::caching_device_vector - left_counts; // Useful to keep a bunch of zeroed memory for sort position - std::vector streams; + left_counts_; // Useful to keep a bunch of zeroed memory for sort position + std::vector streams_; public: RowPartitioner(int device_idx, size_t num_rows); @@ -108,19 +108,19 @@ class RowPartitioner { template void UpdatePosition(bst_node_t nidx, bst_node_t left_nidx, bst_node_t right_nidx, UpdatePositionOpT op) { - dh::safe_cuda(cudaSetDevice(device_idx)); - Segment segment = ridx_segments.at(nidx); // rows belongs to node nidx - auto d_ridx = ridx.CurrentSpan(); - auto d_position = position.CurrentSpan(); - if (left_counts.size() <= nidx) { - left_counts.resize((nidx * 2) + 1); - thrust::fill(left_counts.begin(), left_counts.end(), 0); + dh::safe_cuda(cudaSetDevice(device_idx_)); + Segment segment = ridx_segments_.at(nidx); // rows belongs to node nidx + auto d_ridx = ridx_.CurrentSpan(); + auto d_position = position_.CurrentSpan(); + if (left_counts_.size() <= nidx) { + left_counts_.resize((nidx * 2) + 1); + thrust::fill(left_counts_.begin(), left_counts_.end(), 0); } // Now we divide the row segment into left and right node. - int64_t* d_left_count = left_counts.data().get() + nidx; + int64_t* d_left_count = left_counts_.data().get() + nidx; // Launch 1 thread for each row - dh::LaunchN<1, 128>(device_idx, segment.Size(), [=] __device__(size_t idx) { + dh::LaunchN<1, 128>(device_idx_, segment.Size(), [=] __device__(size_t idx) { // LaunchN starts from zero, so we restore the row index by adding segment.begin idx += segment.begin; RowIndexT ridx = d_ridx[idx]; @@ -132,19 +132,19 @@ class RowPartitioner { // Overlap device to host memory copy (left_count) with sort int64_t left_count; dh::safe_cuda(cudaMemcpyAsync(&left_count, d_left_count, sizeof(int64_t), - cudaMemcpyDeviceToHost, streams[0])); + cudaMemcpyDeviceToHost, streams_[0])); SortPositionAndCopy(segment, left_nidx, right_nidx, d_left_count, - streams[1]); + streams_[1]); - dh::safe_cuda(cudaStreamSynchronize(streams[0])); + dh::safe_cuda(cudaStreamSynchronize(streams_[0])); CHECK_LE(left_count, segment.Size()); CHECK_GE(left_count, 0); - ridx_segments.resize(std::max(int(ridx_segments.size()), - std::max(left_nidx, right_nidx) + 1)); - ridx_segments[left_nidx] = + ridx_segments_.resize(std::max(static_cast(ridx_segments_.size()), + std::max(left_nidx, right_nidx) + 1)); + ridx_segments_[left_nidx] = Segment(segment.begin, segment.begin + left_count); - ridx_segments[right_nidx] = + ridx_segments_[right_nidx] = Segment(segment.begin + left_count, segment.end); } @@ -159,9 +159,9 @@ class RowPartitioner { */ template void FinalisePosition(FinalisePositionOpT op) { - auto d_position = position.Current(); - const auto d_ridx = ridx.Current(); - dh::LaunchN(device_idx, position.Size(), [=] __device__(size_t idx) { + auto d_position = position_.Current(); + const auto d_ridx = ridx_.Current(); + dh::LaunchN(device_idx_, position_.Size(), [=] __device__(size_t idx) { auto position = d_position[idx]; RowIndexT ridx = d_ridx[idx]; bst_node_t new_position = op(ridx, position); @@ -189,10 +189,10 @@ class RowPartitioner { /** \brief Used to demarcate a contiguous set of row indices associated with * some tree node. */ struct Segment { - size_t begin; - size_t end; + size_t begin { 0 }; + size_t end { 0 }; - Segment() : begin{0}, end{0} {} + Segment() = default; Segment(size_t begin, size_t end) : begin(begin), end(end) { CHECK_GE(end, begin); diff --git a/src/tree/param.h b/src/tree/param.h index 97ae94cda..35eeec111 100644 --- a/src/tree/param.h +++ b/src/tree/param.h @@ -319,9 +319,9 @@ XGBOOST_DEVICE inline float CalcWeight(const TrainingParams &p, GpairT sum_grad) /*! \brief core statistics used for tree construction */ struct XGBOOST_ALIGNAS(16) GradStats { /*! \brief sum gradient statistics */ - double sum_grad; + double sum_grad { 0 }; /*! \brief sum hessian statistics */ - double sum_hess; + double sum_hess { 0 }; public: XGBOOST_DEVICE double GetGrad() const { return sum_grad; } @@ -332,7 +332,7 @@ struct XGBOOST_ALIGNAS(16) GradStats { return os; } - XGBOOST_DEVICE GradStats() : sum_grad{0}, sum_hess{0} { + XGBOOST_DEVICE GradStats() { static_assert(sizeof(GradStats) == 16, "Size of GradStats is not 16 bytes."); } diff --git a/src/tree/tree_model.cc b/src/tree/tree_model.cc index 65544e9cd..61717fc43 100644 --- a/src/tree/tree_model.cc +++ b/src/tree/tree_model.cc @@ -87,7 +87,7 @@ class TreeGenerator { auto const split_index = tree[nid].SplitIndex(); std::string result; if (split_index < fmap_.Size()) { - switch (fmap_.type(split_index)) { + switch (fmap_.TypeOf(split_index)) { case FeatureMap::kIndicator: { result = this->Indicator(tree, nid, depth); break; @@ -536,7 +536,7 @@ class GraphvizGenerator : public TreeGenerator { " {nid} [ label=\"{fname}{<}{cond}\" {params}]\n"; // Indicator only has fname. - bool has_less = (split >= fmap_.Size()) || fmap_.type(split) != FeatureMap::kIndicator; + bool has_less = (split >= fmap_.Size()) || fmap_.TypeOf(split) != FeatureMap::kIndicator; std::string result = SuperT::Match(kNodeTemplate, { {"{nid}", std::to_string(nid)}, {"{fname}", split < fmap_.Size() ? fmap_.Name(split) : @@ -674,7 +674,7 @@ void RegTree::Save(dmlc::Stream* fo) const { } void RegTree::LoadModel(Json const& in) { - fromJson(in["tree_param"], ¶m); + FromJson(in["tree_param"], ¶m); auto n_nodes = param.num_nodes; CHECK_NE(n_nodes, 0); // stats @@ -742,7 +742,7 @@ void RegTree::SaveModel(Json* p_out) const { auto& out = *p_out; CHECK_EQ(param.num_nodes, static_cast(nodes_.size())); CHECK_EQ(param.num_nodes, static_cast(stats_.size())); - out["tree_param"] = toJson(param); + out["tree_param"] = ToJson(param); CHECK_EQ(get(out["tree_param"]["num_nodes"]), std::to_string(param.num_nodes)); using I = Integer::Int; auto n_nodes = param.num_nodes; diff --git a/src/tree/updater_basemaker-inl.h b/src/tree/updater_basemaker-inl.h index 8761e3990..66ab91982 100644 --- a/src/tree/updater_basemaker-inl.h +++ b/src/tree/updater_basemaker-inl.h @@ -40,11 +40,11 @@ class BaseMaker: public TreeUpdater { void LoadConfig(Json const& in) override { auto const& config = get(in); - fromJson(config.at("train_param"), &this->param_); + FromJson(config.at("train_param"), &this->param_); } void SaveConfig(Json* p_out) const override { auto& out = *p_out; - out["train_param"] = toJson(param_); + out["train_param"] = ToJson(param_); } protected: diff --git a/src/tree/updater_colmaker.cc b/src/tree/updater_colmaker.cc index 965922136..7aa738461 100644 --- a/src/tree/updater_colmaker.cc +++ b/src/tree/updater_colmaker.cc @@ -64,13 +64,13 @@ class ColMaker: public TreeUpdater { void LoadConfig(Json const& in) override { auto const& config = get(in); - fromJson(config.at("train_param"), &this->param_); - fromJson(config.at("colmaker_train_param"), &this->colmaker_param_); + FromJson(config.at("train_param"), &this->param_); + FromJson(config.at("colmaker_train_param"), &this->colmaker_param_); } void SaveConfig(Json* p_out) const override { auto& out = *p_out; - out["train_param"] = toJson(param_); - out["colmaker_train_param"] = toJson(colmaker_param_); + out["train_param"] = ToJson(param_); + out["colmaker_train_param"] = ToJson(colmaker_param_); } char const* Name() const override { @@ -134,23 +134,23 @@ class ColMaker: public TreeUpdater { /*! \brief statistics of data */ GradStats stats; /*! \brief last feature value scanned */ - bst_float last_fvalue; + bst_float last_fvalue { 0 }; /*! \brief current best solution */ SplitEntry best; // constructor - ThreadEntry() : last_fvalue{0} {} + ThreadEntry() = default; }; struct NodeEntry { /*! \brief statics for node entry */ GradStats stats; /*! \brief loss of this node, without split */ - bst_float root_gain; + bst_float root_gain { 0.0f }; /*! \brief weight calculated related to current data */ - bst_float weight; + bst_float weight { 0.0f }; /*! \brief current best solution */ SplitEntry best; // constructor - NodeEntry() : root_gain{0.0f}, weight{0.0f} {} + NodeEntry() = default; }; // actual builder that runs the algorithm class Builder { diff --git a/src/tree/updater_gpu_common.cuh b/src/tree/updater_gpu_common.cuh index 81ce1819e..87008f8da 100644 --- a/src/tree/updater_gpu_common.cuh +++ b/src/tree/updater_gpu_common.cuh @@ -53,16 +53,15 @@ enum DefaultDirection { }; struct DeviceSplitCandidate { - float loss_chg; - DefaultDirection dir; - int findex; - float fvalue; + float loss_chg {-FLT_MAX}; + DefaultDirection dir {kLeftDir}; + int findex {-1}; + float fvalue {0}; GradientPair left_sum; GradientPair right_sum; - XGBOOST_DEVICE DeviceSplitCandidate() - : loss_chg(-FLT_MAX), dir(kLeftDir), fvalue(0), findex(-1) {} + XGBOOST_DEVICE DeviceSplitCandidate() {} // NOLINT template XGBOOST_DEVICE void Update(const DeviceSplitCandidate& other, @@ -105,7 +104,7 @@ struct DeviceSplitCandidate { struct DeviceSplitCandidateReduceOp { GPUTrainingParam param; - DeviceSplitCandidateReduceOp(GPUTrainingParam param) : param(param) {} + explicit DeviceSplitCandidateReduceOp(GPUTrainingParam param) : param(std::move(param)) {} XGBOOST_DEVICE DeviceSplitCandidate operator()( const DeviceSplitCandidate& a, const DeviceSplitCandidate& b) const { DeviceSplitCandidate best; @@ -117,38 +116,26 @@ struct DeviceSplitCandidateReduceOp { struct DeviceNodeStats { GradientPair sum_gradients; - float root_gain; - float weight; + float root_gain {-FLT_MAX}; + float weight {-FLT_MAX}; /** default direction for missing values */ - DefaultDirection dir; + DefaultDirection dir {kLeftDir}; /** threshold value for comparison */ - float fvalue; + float fvalue {0.0f}; GradientPair left_sum; GradientPair right_sum; /** \brief The feature index. */ - int fidx; + int fidx{kUnusedNode}; /** node id (used as key for reduce/scan) */ - NodeIdT idx; + NodeIdT idx{kUnusedNode}; - HOST_DEV_INLINE DeviceNodeStats() - : sum_gradients(), - root_gain(-FLT_MAX), - weight(-FLT_MAX), - dir(kLeftDir), - fvalue(0.f), - left_sum(), - right_sum(), - fidx(kUnusedNode), - idx(kUnusedNode) {} + XGBOOST_DEVICE DeviceNodeStats() {} // NOLINT template HOST_DEV_INLINE DeviceNodeStats(GradientPair sum_gradients, NodeIdT nidx, const ParamT& param) : sum_gradients(sum_gradients), - dir(kLeftDir), - fvalue(0.f), - fidx(kUnusedNode), idx(nidx) { this->root_gain = CalcGain(param, sum_gradients.GetGrad(), sum_gradients.GetHess()); diff --git a/src/tree/updater_gpu_hist.cu b/src/tree/updater_gpu_hist.cu index 36400973d..65085bb70 100644 --- a/src/tree/updater_gpu_hist.cu +++ b/src/tree/updater_gpu_hist.cu @@ -628,7 +628,7 @@ struct GPUHistMakerDevice { auto d_node_hist_histogram = hist.GetNodeHistogram(nidx_histogram); auto d_node_hist_subtraction = hist.GetNodeHistogram(nidx_subtraction); - dh::LaunchN(device_id, page->cuts_.TotalBins(), [=] __device__(size_t idx) { + dh::LaunchN(device_id, page->Cuts().TotalBins(), [=] __device__(size_t idx) { d_node_hist_subtraction[idx] = d_node_hist_parent[idx] - d_node_hist_histogram[idx]; }); @@ -756,7 +756,7 @@ struct GPUHistMakerDevice { reducer->AllReduceSum( reinterpret_cast(d_node_hist), reinterpret_cast(d_node_hist), - page->cuts_.TotalBins() * (sizeof(GradientSumT) / sizeof(typename GradientSumT::ValueT))); + page->Cuts().TotalBins() * (sizeof(GradientSumT) / sizeof(typename GradientSumT::ValueT))); reducer->Synchronize(); monitor.StopCuda("AllReduce"); @@ -945,20 +945,20 @@ inline void GPUHistMakerDevice::InitHistogram() { // check if we can use shared memory for building histograms // (assuming atleast we need 2 CTAs per SM to maintain decent latency // hiding) - auto histogram_size = sizeof(GradientSumT) * page->cuts_.TotalBins(); + auto histogram_size = sizeof(GradientSumT) * page->Cuts().TotalBins(); auto max_smem = dh::MaxSharedMemory(device_id); if (histogram_size <= max_smem) { use_shared_memory_histograms = true; } // Init histogram - hist.Init(device_id, page->cuts_.TotalBins()); + hist.Init(device_id, page->Cuts().TotalBins()); } template class GPUHistMakerSpecialised { public: - GPUHistMakerSpecialised() : initialised_{false}, p_last_fmat_{nullptr} {} + GPUHistMakerSpecialised() = default; void Configure(const Args& args, GenericParameter const* generic_param) { param_.UpdateAllowUnknown(args); generic_param_ = generic_param; @@ -1002,7 +1002,7 @@ class GPUHistMakerSpecialised { device_ = generic_param_->gpu_id; CHECK_GE(device_, 0) << "Must have at least one device"; info_ = &dmat->Info(); - reducer_.Init({device_}); + reducer_.Init({device_}); // NOLINT // Synchronise the column sampling seed uint32_t column_sampling_seed = common::GlobalRandom()(); @@ -1083,14 +1083,14 @@ class GPUHistMakerSpecialised { std::unique_ptr> maker; // NOLINT private: - bool initialised_; + bool initialised_ { false }; GPUHistMakerTrainParam hist_maker_param_; GenericParameter const* generic_param_; dh::AllReducer reducer_; - DMatrix* p_last_fmat_; + DMatrix* p_last_fmat_ { nullptr }; int device_{-1}; common::Monitor monitor_; @@ -1123,22 +1123,22 @@ class GPUHistMaker : public TreeUpdater { void LoadConfig(Json const& in) override { auto const& config = get(in); - fromJson(config.at("gpu_hist_train_param"), &this->hist_maker_param_); + FromJson(config.at("gpu_hist_train_param"), &this->hist_maker_param_); if (hist_maker_param_.single_precision_histogram) { float_maker_.reset(new GPUHistMakerSpecialised()); - fromJson(config.at("train_param"), &float_maker_->param_); + FromJson(config.at("train_param"), &float_maker_->param_); } else { double_maker_.reset(new GPUHistMakerSpecialised()); - fromJson(config.at("train_param"), &double_maker_->param_); + FromJson(config.at("train_param"), &double_maker_->param_); } } void SaveConfig(Json* p_out) const override { auto& out = *p_out; - out["gpu_hist_train_param"] = toJson(hist_maker_param_); + out["gpu_hist_train_param"] = ToJson(hist_maker_param_); if (hist_maker_param_.single_precision_histogram) { - out["train_param"] = toJson(float_maker_->param_); + out["train_param"] = ToJson(float_maker_->param_); } else { - out["train_param"] = toJson(double_maker_->param_); + out["train_param"] = ToJson(double_maker_->param_); } } diff --git a/src/tree/updater_prune.cc b/src/tree/updater_prune.cc index 5707b809c..5a31a90dd 100644 --- a/src/tree/updater_prune.cc +++ b/src/tree/updater_prune.cc @@ -38,11 +38,11 @@ class TreePruner: public TreeUpdater { void LoadConfig(Json const& in) override { auto const& config = get(in); - fromJson(config.at("train_param"), &this->param_); + FromJson(config.at("train_param"), &this->param_); } void SaveConfig(Json* p_out) const override { auto& out = *p_out; - out["train_param"] = toJson(param_); + out["train_param"] = ToJson(param_); } bool CanModifyTree() const override { return true; diff --git a/src/tree/updater_quantile_hist.cc b/src/tree/updater_quantile_hist.cc index e17f05cff..3e077eed3 100644 --- a/src/tree/updater_quantile_hist.cc +++ b/src/tree/updater_quantile_hist.cc @@ -565,7 +565,7 @@ void QuantileHistMaker::Builder::InitData(const GHistIndexMatrix& gmat, } hist_builder_ = GHistBuilder(this->nthread_, nbins); - std::vector& row_indices = row_set_collection_.row_indices_; + std::vector& row_indices = *row_set_collection_.Data(); row_indices.resize(info.num_row_); auto* p_row_indices = row_indices.data(); // mark subsample and build list of member rows @@ -978,15 +978,15 @@ void QuantileHistMaker::Builder::ApplySplit(const std::vector nodes common::ParallelFor2d(space, this->nthread_, [&](size_t node_in_set, common::Range1d r) { const int32_t nid = nodes[node_in_set].nid; switch (column_matrix.GetTypeSize()) { - case common::UINT8_BINS_TYPE_SIZE: + case common::kUint8BinsTypeSize: PartitionKernel(node_in_set, nid, r, split_conditions[node_in_set], column_matrix, *p_tree); break; - case common::UINT16_BINS_TYPE_SIZE: + case common::kUint16BinsTypeSize: PartitionKernel(node_in_set, nid, r, split_conditions[node_in_set], column_matrix, *p_tree); break; - case common::UINT32_BINS_TYPE_SIZE: + case common::kUint32BinsTypeSize: PartitionKernel(node_in_set, nid, r, split_conditions[node_in_set], column_matrix, *p_tree); break; diff --git a/src/tree/updater_quantile_hist.h b/src/tree/updater_quantile_hist.h index dfcc4dce8..5d0e09772 100644 --- a/src/tree/updater_quantile_hist.h +++ b/src/tree/updater_quantile_hist.h @@ -81,7 +81,7 @@ using xgboost::common::Column; /*! \brief construct a tree using quantized feature values */ class QuantileHistMaker: public TreeUpdater { public: - QuantileHistMaker() {} + QuantileHistMaker() = default; void Configure(const Args& args) override; void Update(HostDeviceVector* gpair, @@ -93,11 +93,11 @@ class QuantileHistMaker: public TreeUpdater { void LoadConfig(Json const& in) override { auto const& config = get(in); - fromJson(config.at("train_param"), &this->param_); + FromJson(config.at("train_param"), &this->param_); } void SaveConfig(Json* p_out) const override { auto& out = *p_out; - out["train_param"] = toJson(param_); + out["train_param"] = ToJson(param_); } char const* Name() const override { @@ -141,7 +141,8 @@ class QuantileHistMaker: public TreeUpdater { FeatureInteractionConstraintHost int_constraints_, DMatrix const* fmat) : param_(param), pruner_(std::move(pruner)), - spliteval_(std::move(spliteval)), interaction_constraints_{int_constraints_}, + spliteval_(std::move(spliteval)), + interaction_constraints_{std::move(int_constraints_)}, p_last_tree_(nullptr), p_last_fmat_(fmat) { builder_monitor_.Init("Quantile::Builder"); } diff --git a/src/tree/updater_refresh.cc b/src/tree/updater_refresh.cc index 4c6c256e4..d63d88c80 100644 --- a/src/tree/updater_refresh.cc +++ b/src/tree/updater_refresh.cc @@ -27,11 +27,11 @@ class TreeRefresher: public TreeUpdater { } void LoadConfig(Json const& in) override { auto const& config = get(in); - fromJson(config.at("train_param"), &this->param_); + FromJson(config.at("train_param"), &this->param_); } void SaveConfig(Json* p_out) const override { auto& out = *p_out; - out["train_param"] = toJson(param_); + out["train_param"] = ToJson(param_); } char const* Name() const override { return "refresh"; diff --git a/src/tree/updater_skmaker.cc b/src/tree/updater_skmaker.cc index f51b22374..9d57f50e0 100644 --- a/src/tree/updater_skmaker.cc +++ b/src/tree/updater_skmaker.cc @@ -81,13 +81,13 @@ class SketchMaker: public BaseMaker { // statistics needed in the gradient calculation struct SKStats { /*! \brief sum of all positive gradient */ - double pos_grad; + double pos_grad { 0 }; /*! \brief sum of all negative gradient */ - double neg_grad; + double neg_grad { 0 }; /*! \brief sum of hessian statistics */ - double sum_hess; + double sum_hess { 0 }; - SKStats() : pos_grad{0}, neg_grad{0}, sum_hess{0} {} + SKStats() = default; // accumulate statistics void Add(const GradientPair& gpair) { diff --git a/tests/ci_build/Dockerfile.clang_tidy b/tests/ci_build/Dockerfile.clang_tidy index 18941999f..3003ad134 100644 --- a/tests/ci_build/Dockerfile.clang_tidy +++ b/tests/ci_build/Dockerfile.clang_tidy @@ -7,14 +7,19 @@ ENV DEBIAN_FRONTEND noninteractive # Install all basic requirements RUN \ apt-get update && \ - apt-get install -y tar unzip wget git build-essential python3 python3-pip llvm-7 clang-tidy-7 clang-7 && \ + apt-get install -y tar unzip wget git build-essential python3 python3-pip software-properties-common \ + apt-transport-https ca-certificates gnupg-agent && \ + wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add - && \ + add-apt-repository -u 'deb http://apt.llvm.org/bionic/ llvm-toolchain-bionic-10 main' && \ + apt-get update && \ + apt-get install -y llvm-10 clang-tidy-10 clang-10 && \ wget -nv -nc https://cmake.org/files/v3.12/cmake-3.12.0-Linux-x86_64.sh --no-check-certificate && \ bash cmake-3.12.0-Linux-x86_64.sh --skip-license --prefix=/usr # Set default clang-tidy version RUN \ - update-alternatives --install /usr/bin/clang-tidy clang-tidy /usr/bin/clang-tidy-7 100 && \ - update-alternatives --install /usr/bin/clang clang /usr/bin/clang-7 100 + update-alternatives --install /usr/bin/clang-tidy clang-tidy /usr/bin/clang-tidy-10 100 && \ + update-alternatives --install /usr/bin/clang clang /usr/bin/clang-10 100 # Install Python packages RUN \ diff --git a/tests/ci_build/tidy.py b/tests/ci_build/tidy.py index 918c97559..1ee86de5a 100755 --- a/tests/ci_build/tidy.py +++ b/tests/ci_build/tidy.py @@ -8,6 +8,7 @@ import os import sys import re import argparse +from time import time def call(args): @@ -16,7 +17,10 @@ def call(args): stdout=subprocess.PIPE, stderr=subprocess.PIPE) error_msg = completed.stdout.decode('utf-8') - matched = re.search('(src|tests)/.*warning:', error_msg, + # `workspace` is a name used in Jenkins CI. Normally we should keep the + # dir as `xgboost`. + matched = re.search('(workspace|xgboost)/.*(src|tests|include)/.*warning:', + error_msg, re.MULTILINE) if matched is None: return_code = 0 @@ -38,6 +42,11 @@ class ClangTidy(object): self.cuda_lint = args.cuda self.use_dmlc_gtest = args.use_dmlc_gtest + if args.tidy_version: + self.exe = 'clang-tidy-' + str(args.tidy_version) + else: + self.exe = 'clang-tidy' + print('Run linter on CUDA: ', self.cuda_lint) print('Run linter on C++:', self.cpp_lint) print('Use dmlc gtest:', self.use_dmlc_gtest) @@ -49,6 +58,7 @@ class ClangTidy(object): self.cdb_path = os.path.join(self.root_path, 'cdb') def __enter__(self): + self.start = time() if os.path.exists(self.cdb_path): shutil.rmtree(self.cdb_path) self._generate_cdb() @@ -57,6 +67,8 @@ class ClangTidy(object): def __exit__(self, *args): if os.path.exists(self.cdb_path): shutil.rmtree(self.cdb_path) + self.end = time() + print('Finish running clang-tidy:', self.end - self.start) def _generate_cdb(self): '''Run CMake to generate compilation database.''' @@ -84,7 +96,7 @@ class ClangTidy(object): # check each component in a command converted_components = [compiler] - for i in range(len(components)): + for i in range(1, len(components)): if components[i] == '-lineinfo': continue elif components[i] == '-fuse-ld=gold': @@ -119,6 +131,8 @@ class ClangTidy(object): else: converted_components.append(components[i]) + converted_components.append('-isystem /usr/local/cuda/include/') + command = '' for c in converted_components: command = command + ' ' + c @@ -126,8 +140,14 @@ class ClangTidy(object): return command def _configure_flags(self, path, command): - common_args = ['clang-tidy', - "-header-filter='(xgboost\\/src|xgboost\\/include)'", + src = os.path.join(self.root_path, 'src') + src = src.replace('/', '\\/') + include = os.path.join(self.root_path, 'include') + include = include.replace('/', '\\/') + + header_filter = '(' + src + '|' + include + ')' + common_args = [self.exe, + "-header-filter=" + header_filter, '-config='+self.clang_tidy] common_args.append(path) common_args.append('--') @@ -166,6 +186,7 @@ class ClangTidy(object): isxgb = isxgb and path.find('dmlc-core') == -1 isxgb = isxgb and (not path.startswith(self.cdb_path)) if isxgb: + print(path) return True cdb_file = os.path.join(self.cdb_path, 'compile_commands.json') @@ -179,7 +200,6 @@ class ClangTidy(object): for entry in self.compile_commands: path = entry['file'] if should_lint(path): - print(path) args = self._configure_flags(path, entry['command']) all_files.extend(args) return all_files @@ -191,7 +211,7 @@ class ClangTidy(object): BAR = '-'*32 with Pool(cpu_count()) as pool: results = pool.map(call, all_files) - for (process_status, tidy_status, msg) in results: + for i, (process_status, tidy_status, msg) in enumerate(results): # Don't enforce clang-tidy to pass for now due to namespace # for cub in thrust is not correct. if tidy_status == 1: @@ -202,11 +222,12 @@ class ClangTidy(object): 'Message:\n', msg, BAR, '\n') if not passed: - print('Please correct clang-tidy warnings.') + print('Errors in `thrust` namespace can be safely ignored.', + 'Please address rest of the clang-tidy warnings.') return passed -def test_tidy(): +def test_tidy(args): '''See if clang-tidy and our regex is working correctly. There are many subtleties we need to be careful. For instances: @@ -233,7 +254,11 @@ right keywords? tidy_config = fd.read() tidy_config = str(tidy_config) tidy_config = '-config='+tidy_config - args = ['clang-tidy', tidy_config, test_file_path] + if not args.tidy_version: + tidy = 'clang-tidy' + else: + tidy = 'clang-tidy-' + str(args.tidy_version) + args = [tidy, tidy_config, test_file_path] (proc_code, tidy_status, error_msg) = call(args) assert proc_code == 0 assert tidy_status == 1 @@ -243,12 +268,14 @@ right keywords? if __name__ == '__main__': parser = argparse.ArgumentParser(description='Run clang-tidy.') parser.add_argument('--cpp', type=int, default=1) + parser.add_argument('--tidy-version', type=int, default=None, + help='Specify the version of preferred clang-tidy.') parser.add_argument('--cuda', type=int, default=1) parser.add_argument('--use-dmlc-gtest', type=int, default=1, help='Whether to use gtest bundled in dmlc-core.') args = parser.parse_args() - test_tidy() + test_tidy(args) with ClangTidy(args) as linter: passed = linter.run() diff --git a/tests/cpp/c_api/test_c_api.cc b/tests/cpp/c_api/test_c_api.cc index 835d8fd46..164acd8dd 100644 --- a/tests/cpp/c_api/test_c_api.cc +++ b/tests/cpp/c_api/test_c_api.cc @@ -11,7 +11,7 @@ #include "../../../src/common/io.h" -TEST(c_api, XGDMatrixCreateFromMatDT) { +TEST(CAPI, XGDMatrixCreateFromMatDT) { std::vector col0 = {0, -1, 3}; std::vector col1 = {-4.0f, 2.0f, 0.0f}; const char *col0_type = "int32"; @@ -38,7 +38,7 @@ TEST(c_api, XGDMatrixCreateFromMatDT) { delete dmat; } -TEST(c_api, XGDMatrixCreateFromMat_omp) { +TEST(CAPI, XGDMatrixCreateFromMatOmp) { std::vector num_rows = {100, 11374, 15000}; for (auto row : num_rows) { int num_cols = 50; @@ -74,13 +74,13 @@ TEST(c_api, XGDMatrixCreateFromMat_omp) { namespace xgboost { -TEST(c_api, Version) { +TEST(CAPI, Version) { int patch {0}; XGBoostVersion(NULL, NULL, &patch); // NOLINT ASSERT_EQ(patch, XGBOOST_VER_PATCH); } -TEST(c_api, ConfigIO) { +TEST(CAPI, ConfigIO) { size_t constexpr kRows = 10; auto p_dmat = RandomDataGenerator(kRows, 10, 0).GenerateDMatix(); std::vector> mat {p_dmat}; @@ -111,7 +111,7 @@ TEST(c_api, ConfigIO) { ASSERT_EQ(config_0, config_1); } -TEST(c_api, JsonModelIO) { +TEST(CAPI, JsonModelIO) { size_t constexpr kRows = 10; dmlc::TemporaryDirectory tempdir; diff --git a/tests/cpp/common/test_bitfield.cu b/tests/cpp/common/test_bitfield.cu index d641debd8..98fbd2ad1 100644 --- a/tests/cpp/common/test_bitfield.cu +++ b/tests/cpp/common/test_bitfield.cu @@ -27,7 +27,7 @@ TEST(BitField, StorageSize) { ASSERT_EQ(2, size); } -TEST(BitField, GPU_Set) { +TEST(BitField, GPUSet) { dh::device_vector storage; uint32_t constexpr kBits = 128; storage.resize(128); @@ -49,7 +49,7 @@ __global__ void TestOrKernel(LBitField64 lhs, LBitField64 rhs) { lhs |= rhs; } -TEST(BitField, GPU_And) { +TEST(BitField, GPUAnd) { uint32_t constexpr kBits = 128; dh::device_vector lhs_storage(kBits); dh::device_vector rhs_storage(kBits); diff --git a/tests/cpp/common/test_column_matrix.cc b/tests/cpp/common/test_column_matrix.cc index 083aa4abc..7b16279d6 100644 --- a/tests/cpp/common/test_column_matrix.cc +++ b/tests/cpp/common/test_column_matrix.cc @@ -22,19 +22,19 @@ TEST(DenseColumn, Test) { for (auto i = 0ull; i < dmat->Info().num_row_; i++) { for (auto j = 0ull; j < dmat->Info().num_col_; j++) { switch (column_matrix.GetTypeSize()) { - case UINT8_BINS_TYPE_SIZE: { + case kUint8BinsTypeSize: { auto col = column_matrix.GetColumn(j); ASSERT_EQ(gmat.index[i * dmat->Info().num_col_ + j], (*col.get()).GetGlobalBinIdx(i)); } break; - case UINT16_BINS_TYPE_SIZE: { + case kUint16BinsTypeSize: { auto col = column_matrix.GetColumn(j); ASSERT_EQ(gmat.index[i * dmat->Info().num_col_ + j], (*col.get()).GetGlobalBinIdx(i)); } break; - case UINT32_BINS_TYPE_SIZE: { + case kUint32BinsTypeSize: { auto col = column_matrix.GetColumn(j); ASSERT_EQ(gmat.index[i * dmat->Info().num_col_ + j], (*col.get()).GetGlobalBinIdx(i)); @@ -49,7 +49,7 @@ TEST(DenseColumn, Test) { template inline void CheckSparseColumn(const Column& col_input, const GHistIndexMatrix& gmat) { const SparseColumn& col = static_cast& >(col_input); - ASSERT_EQ(col.Size(), gmat.index.size()); + ASSERT_EQ(col.Size(), gmat.index.Size()); for (auto i = 0ull; i < col.Size(); i++) { ASSERT_EQ(gmat.index[gmat.row_ptr[col.GetRowIdx(i)]], col.GetGlobalBinIdx(i)); @@ -67,17 +67,17 @@ TEST(SparseColumn, Test) { ColumnMatrix column_matrix; column_matrix.Init(gmat, 0.5); switch (column_matrix.GetTypeSize()) { - case UINT8_BINS_TYPE_SIZE: { + case kUint8BinsTypeSize: { auto col = column_matrix.GetColumn(0); CheckSparseColumn(*col.get(), gmat); } break; - case UINT16_BINS_TYPE_SIZE: { + case kUint16BinsTypeSize: { auto col = column_matrix.GetColumn(0); CheckSparseColumn(*col.get(), gmat); } break; - case UINT32_BINS_TYPE_SIZE: { + case kUint32BinsTypeSize: { auto col = column_matrix.GetColumn(0); CheckSparseColumn(*col.get(), gmat); } @@ -108,17 +108,17 @@ TEST(DenseColumnWithMissing, Test) { ColumnMatrix column_matrix; column_matrix.Init(gmat, 0.2); switch (column_matrix.GetTypeSize()) { - case UINT8_BINS_TYPE_SIZE: { + case kUint8BinsTypeSize: { auto col = column_matrix.GetColumn(0); CheckColumWithMissingValue(*col.get(), gmat); } break; - case UINT16_BINS_TYPE_SIZE: { + case kUint16BinsTypeSize: { auto col = column_matrix.GetColumn(0); CheckColumWithMissingValue(*col.get(), gmat); } break; - case UINT32_BINS_TYPE_SIZE: { + case kUint32BinsTypeSize: { auto col = column_matrix.GetColumn(0); CheckColumWithMissingValue(*col.get(), gmat); } diff --git a/tests/cpp/common/test_device_helpers.cu b/tests/cpp/common/test_device_helpers.cu index c2293a78a..cb69c24f1 100644 --- a/tests/cpp/common/test_device_helpers.cu +++ b/tests/cpp/common/test_device_helpers.cu @@ -55,14 +55,14 @@ void TestLbs() { } } -TEST(cub_lbs, Test) { +TEST(CubLBS, Test) { TestLbs(); } -TEST(sumReduce, Test) { +TEST(SumReduce, Test) { thrust::device_vector data(100, 1.0f); dh::CubMemory temp; - auto sum = dh::SumReduction(temp, dh::Raw(data), data.size()); + auto sum = dh::SumReduction(&temp, dh::Raw(data), data.size()); ASSERT_NEAR(sum, 100.0f, 1e-5); } @@ -81,7 +81,7 @@ void TestAllocator() { } // Define the test in a function so we can use device lambda -TEST(bulkAllocator, Test) { +TEST(BulkAllocator, Test) { TestAllocator(); } diff --git a/tests/cpp/common/test_group_data.cc b/tests/cpp/common/test_group_data.cc index d71315999..94bb23e4a 100644 --- a/tests/cpp/common/test_group_data.cc +++ b/tests/cpp/common/test_group_data.cc @@ -8,7 +8,7 @@ namespace xgboost { namespace common { -TEST(group_data, ParallelGroupBuilder) { +TEST(GroupData, ParallelGroupBuilder) { std::vector offsets; std::vector data; ParallelGroupBuilder builder(&offsets, &data); diff --git a/tests/cpp/common/test_hist_util.cc b/tests/cpp/common/test_hist_util.cc index a6fe02db0..8b7b93be6 100644 --- a/tests/cpp/common/test_hist_util.cc +++ b/tests/cpp/common/test_hist_util.cc @@ -218,7 +218,7 @@ TEST(SparseCuts, MultiThreadedBuild) { omp_set_num_threads(ori_nthreads); } -TEST(hist_util, DenseCutsCategorical) { +TEST(HistUtil, DenseCutsCategorical) { int categorical_sizes[] = {2, 6, 8, 12}; int num_bins = 256; int sizes[] = {25, 100, 1000}; @@ -240,7 +240,7 @@ TEST(hist_util, DenseCutsCategorical) { } } -TEST(hist_util, DenseCutsAccuracyTest) { +TEST(HistUtil, DenseCutsAccuracyTest) { int bin_sizes[] = {2, 16, 256, 512}; int sizes[] = {100, 1000, 1500}; int num_columns = 5; @@ -256,7 +256,7 @@ TEST(hist_util, DenseCutsAccuracyTest) { } } -TEST(hist_util, DenseCutsAccuracyTestWeights) { +TEST(HistUtil, DenseCutsAccuracyTestWeights) { int bin_sizes[] = {2, 16, 256, 512}; int sizes[] = {100, 1000, 1500}; int num_columns = 5; @@ -274,7 +274,7 @@ TEST(hist_util, DenseCutsAccuracyTestWeights) { } } -TEST(hist_util, DenseCutsExternalMemory) { +TEST(HistUtil, DenseCutsExternalMemory) { int bin_sizes[] = {2, 16, 256, 512}; int sizes[] = {100, 1000, 1500}; int num_columns = 5; @@ -292,7 +292,7 @@ TEST(hist_util, DenseCutsExternalMemory) { } } -TEST(hist_util, SparseCutsAccuracyTest) { +TEST(HistUtil, SparseCutsAccuracyTest) { int bin_sizes[] = {2, 16, 256, 512}; int sizes[] = {100, 1000, 1500}; int num_columns = 5; @@ -308,7 +308,7 @@ TEST(hist_util, SparseCutsAccuracyTest) { } } -TEST(hist_util, SparseCutsCategorical) { +TEST(HistUtil, SparseCutsCategorical) { int categorical_sizes[] = {2, 6, 8, 12}; int num_bins = 256; int sizes[] = {25, 100, 1000}; @@ -330,7 +330,7 @@ TEST(hist_util, SparseCutsCategorical) { } } -TEST(hist_util, SparseCutsExternalMemory) { +TEST(HistUtil, SparseCutsExternalMemory) { int bin_sizes[] = {2, 16, 256, 512}; int sizes[] = {100, 1000, 1500}; int num_columns = 5; @@ -348,13 +348,13 @@ TEST(hist_util, SparseCutsExternalMemory) { } } -TEST(hist_util, IndexBinBound) { +TEST(HistUtil, IndexBinBound) { uint64_t bin_sizes[] = { static_cast(std::numeric_limits::max()) + 1, static_cast(std::numeric_limits::max()) + 1, static_cast(std::numeric_limits::max()) + 2 }; - BinTypeSize expected_bin_type_sizes[] = {UINT8_BINS_TYPE_SIZE, - UINT16_BINS_TYPE_SIZE, - UINT32_BINS_TYPE_SIZE}; + BinTypeSize expected_bin_type_sizes[] = {kUint8BinsTypeSize, + kUint16BinsTypeSize, + kUint32BinsTypeSize}; size_t constexpr kRows = 100; size_t constexpr kCols = 10; @@ -364,18 +364,18 @@ TEST(hist_util, IndexBinBound) { common::GHistIndexMatrix hmat; hmat.Init(p_fmat.get(), max_bin); - EXPECT_EQ(hmat.index.size(), kRows*kCols); - EXPECT_EQ(expected_bin_type_sizes[bin_id++], hmat.index.getBinTypeSize()); + EXPECT_EQ(hmat.index.Size(), kRows*kCols); + EXPECT_EQ(expected_bin_type_sizes[bin_id++], hmat.index.GetBinTypeSize()); } } -TEST(hist_util, SparseIndexBinBound) { +TEST(HistUtil, SparseIndexBinBound) { uint64_t bin_sizes[] = { static_cast(std::numeric_limits::max()) + 1, static_cast(std::numeric_limits::max()) + 1, static_cast(std::numeric_limits::max()) + 2 }; - BinTypeSize expected_bin_type_sizes[] = { UINT32_BINS_TYPE_SIZE, - UINT32_BINS_TYPE_SIZE, - UINT32_BINS_TYPE_SIZE }; + BinTypeSize expected_bin_type_sizes[] = { kUint32BinsTypeSize, + kUint32BinsTypeSize, + kUint32BinsTypeSize }; size_t constexpr kRows = 100; size_t constexpr kCols = 10; @@ -384,19 +384,19 @@ TEST(hist_util, SparseIndexBinBound) { auto p_fmat = RandomDataGenerator(kRows, kCols, 0.2).GenerateDMatix(); common::GHistIndexMatrix hmat; hmat.Init(p_fmat.get(), max_bin); - EXPECT_EQ(expected_bin_type_sizes[bin_id++], hmat.index.getBinTypeSize()); + EXPECT_EQ(expected_bin_type_sizes[bin_id++], hmat.index.GetBinTypeSize()); } } template void CheckIndexData(T* data_ptr, uint32_t* offsets, const common::GHistIndexMatrix& hmat, size_t n_cols) { - for (size_t i = 0; i < hmat.index.size(); ++i) { + for (size_t i = 0; i < hmat.index.Size(); ++i) { EXPECT_EQ(data_ptr[i] + offsets[i % n_cols], hmat.index[i]); } } -TEST(hist_util, IndexBinData) { +TEST(HistUtil, IndexBinData) { uint64_t constexpr kBinSizes[] = { static_cast(std::numeric_limits::max()) + 1, static_cast(std::numeric_limits::max()) + 1, static_cast(std::numeric_limits::max()) + 2 }; @@ -407,8 +407,8 @@ TEST(hist_util, IndexBinData) { auto p_fmat = RandomDataGenerator(kRows, kCols, 0).GenerateDMatix(); common::GHistIndexMatrix hmat; hmat.Init(p_fmat.get(), max_bin); - uint32_t* offsets = hmat.index.offset(); - EXPECT_EQ(hmat.index.size(), kRows*kCols); + uint32_t* offsets = hmat.index.Offset(); + EXPECT_EQ(hmat.index.Size(), kRows*kCols); switch (max_bin) { case kBinSizes[0]: CheckIndexData(hmat.index.data(), @@ -426,7 +426,7 @@ TEST(hist_util, IndexBinData) { } } -TEST(hist_util, SparseIndexBinData) { +TEST(HistUtil, SparseIndexBinData) { uint64_t bin_sizes[] = { static_cast(std::numeric_limits::max()) + 1, static_cast(std::numeric_limits::max()) + 1, static_cast(std::numeric_limits::max()) + 2 }; @@ -437,10 +437,10 @@ TEST(hist_util, SparseIndexBinData) { auto p_fmat = RandomDataGenerator(kRows, kCols, 0.2).GenerateDMatix(); common::GHistIndexMatrix hmat; hmat.Init(p_fmat.get(), max_bin); - EXPECT_EQ(hmat.index.offset(), nullptr); + EXPECT_EQ(hmat.index.Offset(), nullptr); uint32_t* data_ptr = hmat.index.data(); - for (size_t i = 0; i < hmat.index.size(); ++i) { + for (size_t i = 0; i < hmat.index.Size(); ++i) { EXPECT_EQ(data_ptr[i], hmat.index[i]); } } diff --git a/tests/cpp/common/test_hist_util.cu b/tests/cpp/common/test_hist_util.cu index 014a00a04..b728acb68 100644 --- a/tests/cpp/common/test_hist_util.cu +++ b/tests/cpp/common/test_hist_util.cu @@ -32,7 +32,7 @@ HistogramCuts GetHostCuts(AdapterT *adapter, int num_bins, float missing) { builder.Build(&dmat, num_bins); return cuts; } -TEST(hist_util, DeviceSketch) { +TEST(HistUtil, DeviceSketch) { int num_rows = 5; int num_columns = 1; int num_bins = 4; @@ -61,7 +61,7 @@ size_t RequiredSampleCutsTest(int max_bins, size_t num_rows) { return std::min(num_cuts, num_rows); } -TEST(hist_util, DeviceSketchMemory) { +TEST(HistUtil, DeviceSketchMemory) { int num_columns = 100; int num_rows = 1000; int num_bins = 256; @@ -81,7 +81,7 @@ TEST(hist_util, DeviceSketchMemory) { bytes_num_elements + bytes_cuts + bytes_constant); } -TEST(hist_util, DeviceSketchMemoryWeights) { +TEST(HistUtil, DeviceSketchMemoryWeights) { int num_columns = 100; int num_rows = 1000; int num_bins = 256; @@ -102,7 +102,7 @@ TEST(hist_util, DeviceSketchMemoryWeights) { size_t((bytes_num_elements + bytes_cuts) * 1.05)); } -TEST(hist_util, DeviceSketchDeterminism) { +TEST(HistUtil, DeviceSketchDeterminism) { int num_rows = 500; int num_columns = 5; int num_bins = 256; @@ -117,7 +117,7 @@ TEST(hist_util, DeviceSketchDeterminism) { } } - TEST(hist_util, DeviceSketchCategorical) { + TEST(HistUtil, DeviceSketchCategorical) { int categorical_sizes[] = {2, 6, 8, 12}; int num_bins = 256; int sizes[] = {25, 100, 1000}; @@ -131,7 +131,7 @@ TEST(hist_util, DeviceSketchDeterminism) { } } -TEST(hist_util, DeviceSketchMultipleColumns) { +TEST(HistUtil, DeviceSketchMultipleColumns) { int bin_sizes[] = {2, 16, 256, 512}; int sizes[] = {100, 1000, 1500}; int num_columns = 5; @@ -146,7 +146,7 @@ TEST(hist_util, DeviceSketchMultipleColumns) { } -TEST(hist_util, DeviceSketchMultipleColumnsWeights) { +TEST(HistUtil, DeviceSketchMultipleColumnsWeights) { int bin_sizes[] = {2, 16, 256, 512}; int sizes[] = {100, 1000, 1500}; int num_columns = 5; @@ -161,7 +161,7 @@ TEST(hist_util, DeviceSketchMultipleColumnsWeights) { } } -TEST(hist_util, DeviceSketchBatches) { +TEST(HistUtil, DeviceSketchBatches) { int num_bins = 256; int num_rows = 5000; int batch_sizes[] = {0, 100, 1500, 6000}; @@ -174,7 +174,7 @@ TEST(hist_util, DeviceSketchBatches) { } } -TEST(hist_util, DeviceSketchMultipleColumnsExternal) { +TEST(HistUtil, DeviceSketchMultipleColumnsExternal) { int bin_sizes[] = {2, 16, 256, 512}; int sizes[] = {100, 1000, 1500}; int num_columns =5; @@ -190,7 +190,7 @@ TEST(hist_util, DeviceSketchMultipleColumnsExternal) { } } -TEST(hist_util, AdapterDeviceSketch) +TEST(HistUtil, AdapterDeviceSketch) { int rows = 5; int cols = 1; @@ -212,7 +212,7 @@ TEST(hist_util, AdapterDeviceSketch) EXPECT_EQ(device_cuts.MinValues(), host_cuts.MinValues()); } -TEST(hist_util, AdapterDeviceSketchMemory) { +TEST(HistUtil, AdapterDeviceSketchMemory) { int num_columns = 100; int num_rows = 1000; int num_bins = 256; @@ -235,7 +235,7 @@ TEST(hist_util, AdapterDeviceSketchMemory) { bytes_num_elements + bytes_cuts + bytes_num_columns + bytes_constant); } - TEST(hist_util, AdapterDeviceSketchCategorical) { + TEST(HistUtil, AdapterDeviceSketchCategorical) { int categorical_sizes[] = {2, 6, 8, 12}; int num_bins = 256; int sizes[] = {25, 100, 1000}; @@ -252,7 +252,7 @@ TEST(hist_util, AdapterDeviceSketchMemory) { } } -TEST(hist_util, AdapterDeviceSketchMultipleColumns) { +TEST(HistUtil, AdapterDeviceSketchMultipleColumns) { int bin_sizes[] = {2, 16, 256, 512}; int sizes[] = {100, 1000, 1500}; int num_columns = 5; @@ -268,7 +268,7 @@ TEST(hist_util, AdapterDeviceSketchMultipleColumns) { } } } -TEST(hist_util, AdapterDeviceSketchBatches) { +TEST(HistUtil, AdapterDeviceSketchBatches) { int num_bins = 256; int num_rows = 5000; int batch_sizes[] = {0, 100, 1500, 6000}; @@ -287,7 +287,7 @@ TEST(hist_util, AdapterDeviceSketchBatches) { // Check sketching from adapter or DMatrix results in the same answer // Consistency here is useful for testing and user experience -TEST(hist_util, SketchingEquivalent) { +TEST(HistUtil, SketchingEquivalent) { int bin_sizes[] = {2, 16, 256, 512}; int sizes[] = {100, 1000, 1500}; int num_columns = 5; diff --git a/tests/cpp/common/test_host_device_vector.cu b/tests/cpp/common/test_host_device_vector.cu index b483edf39..e5ac339e7 100644 --- a/tests/cpp/common/test_host_device_vector.cu +++ b/tests/cpp/common/test_host_device_vector.cu @@ -176,7 +176,7 @@ TEST(HostDeviceVector, Span) { ASSERT_TRUE(vec.HostCanWrite()); } -TEST(HostDeviceVector, MGPU_Basic) { +TEST(HostDeviceVector, MGPU_Basic) { // NOLINT if (AllVisibleGPUs() < 2) { LOG(WARNING) << "Not testing in multi-gpu environment."; return; diff --git a/tests/cpp/common/test_json.cc b/tests/cpp/common/test_json.cc index c4adbb42e..b636ef7e3 100644 --- a/tests/cpp/common/test_json.cc +++ b/tests/cpp/common/test_json.cc @@ -262,7 +262,7 @@ TEST(Json, Indexing) { Json j {Json::Load(&reader)}; auto& value_1 = j["model_parameter"]; auto& value = value_1["base_score"]; - std::string result = Cast(&value.GetValue())->getString(); + std::string result = Cast(&value.GetValue())->GetString(); ASSERT_EQ(result, "0.5"); } @@ -406,7 +406,7 @@ TEST(Json, WrongCasts) { } } -TEST(Json, Int_vs_Float) { +TEST(Json, IntVSFloat) { // If integer is parsed as float, calling `get()' will throw. { std::string str = R"json( diff --git a/tests/cpp/common/test_transform_range.cu b/tests/cpp/common/test_transform_range.cu index 4e4c259e4..a1958ded5 100644 --- a/tests/cpp/common/test_transform_range.cu +++ b/tests/cpp/common/test_transform_range.cu @@ -5,7 +5,7 @@ namespace xgboost { namespace common { -TEST(Transform, MGPU_SpecifiedGpuId) { +TEST(Transform, MGPU_SpecifiedGpuId) { // NOLINT if (AllVisibleGPUs() < 2) { LOG(WARNING) << "Not testing in multi-gpu environment."; return; diff --git a/tests/cpp/data/test_adapter.cc b/tests/cpp/data/test_adapter.cc index 5ead2064b..04e2fa074 100644 --- a/tests/cpp/data/test_adapter.cc +++ b/tests/cpp/data/test_adapter.cc @@ -67,7 +67,7 @@ TEST(Adapter, CSCAdapterColsMoreThanRows) { EXPECT_EQ(inst[3].index, 3); } -TEST(c_api, DMatrixSliceAdapterFromSimpleDMatrix) { +TEST(CAPI, DMatrixSliceAdapterFromSimpleDMatrix) { auto p_dmat = RandomDataGenerator(6, 2, 1.0).GenerateDMatix(); std::vector ridx_set = {1, 3, 5}; diff --git a/tests/cpp/data/test_device_adapter.cu b/tests/cpp/data/test_device_adapter.cu index 09de9bb88..e652db377 100644 --- a/tests/cpp/data/test_device_adapter.cu +++ b/tests/cpp/data/test_device_adapter.cu @@ -50,6 +50,6 @@ void TestCudfAdapter() }); } -TEST(device_adapter, CudfAdapter) { +TEST(DeviceAdapter, CudfAdapter) { TestCudfAdapter(); } diff --git a/tests/cpp/data/test_device_dmatrix.cu b/tests/cpp/data/test_device_dmatrix.cu index 61ea47542..880779107 100644 --- a/tests/cpp/data/test_device_dmatrix.cu +++ b/tests/cpp/data/test_device_dmatrix.cu @@ -32,7 +32,7 @@ TEST(DeviceDMatrix, RowMajor) { for(auto i = 0ull; i < x.size(); i++) { int column_idx = i % num_columns; - EXPECT_EQ(impl->cuts_.SearchBin(x[i], column_idx), iterator[i]); + EXPECT_EQ(impl->Cuts().SearchBin(x[i], column_idx), iterator[i]); } EXPECT_EQ(dmat.Info().num_col_, num_columns); EXPECT_EQ(dmat.Info().num_row_, num_rows); @@ -93,9 +93,9 @@ TEST(DeviceDMatrix, ColumnMajor) { for (auto i = 0ull; i < kRows; i++) { for (auto j = 0ull; j < columns.size(); j++) { if (j == 0) { - EXPECT_EQ(iterator[i * 2 + j], impl->cuts_.SearchBin(d_data_0[i], j)); + EXPECT_EQ(iterator[i * 2 + j], impl->Cuts().SearchBin(d_data_0[i], j)); } else { - EXPECT_EQ(iterator[i * 2 + j], impl->cuts_.SearchBin(d_data_1[i], j)); + EXPECT_EQ(iterator[i * 2 + j], impl->Cuts().SearchBin(d_data_1[i], j)); } } } @@ -123,7 +123,7 @@ TEST(DeviceDMatrix, Equivalent) { const auto &device_dmat_batch = *device_dmat.GetBatches({0, num_bins}).begin(); - ASSERT_EQ(batch.Impl()->cuts_.Values(), device_dmat_batch.Impl()->cuts_.Values()); + ASSERT_EQ(batch.Impl()->Cuts().Values(), device_dmat_batch.Impl()->Cuts().Values()); ASSERT_EQ(batch.Impl()->gidx_buffer.HostVector(), device_dmat_batch.Impl()->gidx_buffer.HostVector()); } diff --git a/tests/cpp/data/test_ellpack_page.cu b/tests/cpp/data/test_ellpack_page.cu index a713979e3..14a05c09b 100644 --- a/tests/cpp/data/test_ellpack_page.cu +++ b/tests/cpp/data/test_ellpack_page.cu @@ -21,7 +21,7 @@ TEST(EllpackPage, EmptyDMatrix) { auto& page = *dmat->GetBatches({0, kMaxBin}).begin(); auto impl = page.Impl(); ASSERT_EQ(impl->row_stride, 0); - ASSERT_EQ(impl->cuts_.TotalBins(), 0); + ASSERT_EQ(impl->Cuts().TotalBins(), 0); ASSERT_EQ(impl->gidx_buffer.Size(), 4); } @@ -106,7 +106,7 @@ TEST(EllpackPage, Copy) { auto page = (*dmat->GetBatches(param).begin()).Impl(); // Create an empty result page. - EllpackPageImpl result(0, page->cuts_, page->is_dense, page->row_stride, + EllpackPageImpl result(0, page->Cuts(), page->is_dense, page->row_stride, kRows); // Copy batch pages into the result page. @@ -152,7 +152,7 @@ TEST(EllpackPage, Compact) { auto page = (*dmat->GetBatches(param).begin()).Impl(); // Create an empty result page. - EllpackPageImpl result(0, page->cuts_, page->is_dense, page->row_stride, + EllpackPageImpl result(0, page->Cuts(), page->is_dense, page->row_stride, kCompactedRows); // Compact batch pages into the result page. diff --git a/tests/cpp/data/test_sparse_page_dmatrix.cu b/tests/cpp/data/test_sparse_page_dmatrix.cu index aeadec7d6..cf4059d40 100644 --- a/tests/cpp/data/test_sparse_page_dmatrix.cu +++ b/tests/cpp/data/test_sparse_page_dmatrix.cu @@ -63,14 +63,14 @@ TEST(SparsePageDMatrix, EllpackPageContent) { EXPECT_EQ(impl->n_rows, kRows); EXPECT_FALSE(impl->is_dense); EXPECT_EQ(impl->row_stride, 2); - EXPECT_EQ(impl->cuts_.TotalBins(), 4); + EXPECT_EQ(impl->Cuts().TotalBins(), 4); auto impl_ext = (*dmat_ext->GetBatches(param).begin()).Impl(); EXPECT_EQ(impl_ext->base_rowid, 0); EXPECT_EQ(impl_ext->n_rows, kRows); EXPECT_FALSE(impl_ext->is_dense); EXPECT_EQ(impl_ext->row_stride, 2); - EXPECT_EQ(impl_ext->cuts_.TotalBins(), 4); + EXPECT_EQ(impl_ext->Cuts().TotalBins(), 4); std::vector buffer(impl->gidx_buffer.HostVector()); std::vector buffer_ext(impl_ext->gidx_buffer.HostVector()); @@ -149,7 +149,6 @@ TEST(SparsePageDMatrix, EllpackPageMultipleLoops) { dmat_ext(CreateSparsePageDMatrixWithRC(kRows, kCols, kPageSize, true, tmpdir)); BatchParam param{0, kMaxBins, kPageSize}; - auto impl = (*dmat->GetBatches(param).begin()).Impl(); size_t current_row = 0; for (auto& page : dmat_ext->GetBatches(param)) { diff --git a/tests/cpp/linear/test_linear.cc b/tests/cpp/linear/test_linear.cc index d91042794..5fb1bbfef 100644 --- a/tests/cpp/linear/test_linear.cc +++ b/tests/cpp/linear/test_linear.cc @@ -33,7 +33,7 @@ TEST(Linear, Shotgun) { model.LazyInitModel(); updater->Update(&gpair, p_fmat.get(), &model, gpair.Size()); - ASSERT_EQ(model.bias()[0], 5.0f); + ASSERT_EQ(model.Bias()[0], 5.0f); } { @@ -68,7 +68,7 @@ TEST(Linear, coordinate) { model.LazyInitModel(); updater->Update(&gpair, p_fmat.get(), &model, gpair.Size()); - ASSERT_EQ(model.bias()[0], 5.0f); + ASSERT_EQ(model.Bias()[0], 5.0f); } TEST(Coordinate, JsonIO){ diff --git a/tests/cpp/linear/test_linear.cu b/tests/cpp/linear/test_linear.cu index 263b01b67..5829213ea 100644 --- a/tests/cpp/linear/test_linear.cu +++ b/tests/cpp/linear/test_linear.cu @@ -30,7 +30,7 @@ TEST(Linear, GPUCoordinate) { model.LazyInitModel(); updater->Update(&gpair, mat.get(), &model, gpair.Size()); - ASSERT_EQ(model.bias()[0], 5.0f); + ASSERT_EQ(model.Bias()[0], 5.0f); } TEST(GPUCoordinate, JsonIO) { diff --git a/tests/cpp/predictor/test_gpu_predictor.cu b/tests/cpp/predictor/test_gpu_predictor.cu index 7c8dac373..91f5b9ef6 100644 --- a/tests/cpp/predictor/test_gpu_predictor.cu +++ b/tests/cpp/predictor/test_gpu_predictor.cu @@ -126,7 +126,7 @@ TEST(GPUPredictor, InplacePredictCuDF) { TestInplacePrediction(x, "gpu_predictor", kRows, kCols, 0); } -TEST(GPUPredictor, MGPU_InplacePredict) { +TEST(GPUPredictor, MGPU_InplacePredict) { // NOLINT int32_t n_gpus = xgboost::common::AllVisibleGPUs(); if (n_gpus <= 1) { LOG(WARNING) << "GPUPredictor.MGPU_InplacePredict is skipped."; diff --git a/tests/cpp/test_learner.cc b/tests/cpp/test_learner.cc index 5308d9baa..986890639 100644 --- a/tests/cpp/test_learner.cc +++ b/tests/cpp/test_learner.cc @@ -86,7 +86,7 @@ TEST(Learner, CheckGroup) { EXPECT_ANY_THROW(learner->UpdateOneIter(0, p_mat)); } -TEST(Learner, SLOW_CheckMultiBatch) { +TEST(Learner, SLOW_CheckMultiBatch) { // NOLINT // Create sufficiently large data to make two row pages dmlc::TemporaryDirectory tempdir; const std::string tmp_file = tempdir.path + "/big.libsvm"; diff --git a/tests/cpp/test_serialization.cc b/tests/cpp/test_serialization.cc index dc74cd890..cd4cf13f3 100644 --- a/tests/cpp/test_serialization.cc +++ b/tests/cpp/test_serialization.cc @@ -254,7 +254,7 @@ TEST_F(SerializationTest, Hist) { fmap_, p_dmat_); } -TEST_F(SerializationTest, CPU_CoordDescent) { +TEST_F(SerializationTest, CPUCoordDescent) { TestLearnerSerialization({{"booster", "gblinear"}, {"seed", "0"}, {"nthread", "1"}, @@ -264,7 +264,7 @@ TEST_F(SerializationTest, CPU_CoordDescent) { } #if defined(XGBOOST_USE_CUDA) -TEST_F(SerializationTest, GPU_Hist) { +TEST_F(SerializationTest, GPUHist) { TestLearnerSerialization({{"booster", "gbtree"}, {"seed", "0"}, {"enable_experimental_json_serialization", "1"}, @@ -338,7 +338,7 @@ TEST_F(SerializationTest, ConfigurationCount) { xgboost::ConsoleLogger::Configure({{"verbosity", "2"}}); } -TEST_F(SerializationTest, GPU_CoordDescent) { +TEST_F(SerializationTest, GPUCoordDescent) { TestLearnerSerialization({{"booster", "gblinear"}, {"seed", "0"}, {"nthread", "1"}, @@ -431,7 +431,7 @@ TEST_F(LogitSerializationTest, Hist) { fmap_, p_dmat_); } -TEST_F(LogitSerializationTest, CPU_CoordDescent) { +TEST_F(LogitSerializationTest, CPUCoordDescent) { TestLearnerSerialization({{"booster", "gblinear"}, {"seed", "0"}, {"nthread", "1"}, @@ -441,7 +441,7 @@ TEST_F(LogitSerializationTest, CPU_CoordDescent) { } #if defined(XGBOOST_USE_CUDA) -TEST_F(LogitSerializationTest, GPU_Hist) { +TEST_F(LogitSerializationTest, GPUHist) { TestLearnerSerialization({{"booster", "gbtree"}, {"objective", "binary:logistic"}, {"seed", "0"}, @@ -471,7 +471,7 @@ TEST_F(LogitSerializationTest, GPU_Hist) { fmap_, p_dmat_); } -TEST_F(LogitSerializationTest, GPU_CoordDescent) { +TEST_F(LogitSerializationTest, GPUCoordDescent) { TestLearnerSerialization({{"booster", "gblinear"}, {"objective", "binary:logistic"}, {"seed", "0"}, @@ -586,7 +586,7 @@ TEST_F(MultiClassesSerializationTest, Hist) { fmap_, p_dmat_); } -TEST_F(MultiClassesSerializationTest, CPU_CoordDescent) { +TEST_F(MultiClassesSerializationTest, CPUCoordDescent) { TestLearnerSerialization({{"booster", "gblinear"}, {"seed", "0"}, {"nthread", "1"}, @@ -596,7 +596,7 @@ TEST_F(MultiClassesSerializationTest, CPU_CoordDescent) { } #if defined(XGBOOST_USE_CUDA) -TEST_F(MultiClassesSerializationTest, GPU_Hist) { +TEST_F(MultiClassesSerializationTest, GPUHist) { TestLearnerSerialization({{"booster", "gbtree"}, {"num_class", std::to_string(kClasses)}, {"seed", "0"}, @@ -632,7 +632,7 @@ TEST_F(MultiClassesSerializationTest, GPU_Hist) { fmap_, p_dmat_); } -TEST_F(MultiClassesSerializationTest, GPU_CoordDescent) { +TEST_F(MultiClassesSerializationTest, GPUCoordDescent) { TestLearnerSerialization({{"booster", "gblinear"}, {"num_class", std::to_string(kClasses)}, {"seed", "0"}, diff --git a/tests/cpp/tree/gpu_hist/test_gradient_based_sampler.cu b/tests/cpp/tree/gpu_hist/test_gradient_based_sampler.cu index bde427670..183d5b3f1 100644 --- a/tests/cpp/tree/gpu_hist/test_gradient_based_sampler.cu +++ b/tests/cpp/tree/gpu_hist/test_gradient_based_sampler.cu @@ -69,7 +69,7 @@ TEST(GradientBasedSampler, NoSampling) { } // In external mode, when not sampling, we concatenate the pages together. -TEST(GradientBasedSampler, NoSampling_ExternalMemory) { +TEST(GradientBasedSampler, NoSamplingExternalMemory) { constexpr size_t kRows = 2048; constexpr size_t kCols = 1; constexpr float kSubsample = 1.0f; @@ -121,7 +121,7 @@ TEST(GradientBasedSampler, UniformSampling) { VerifySampling(kPageSize, kSubsample, kSamplingMethod, kFixedSizeSampling, kCheckSum); } -TEST(GradientBasedSampler, UniformSampling_ExternalMemory) { +TEST(GradientBasedSampler, UniformSamplingExternalMemory) { constexpr size_t kPageSize = 1024; constexpr float kSubsample = 0.5; constexpr int kSamplingMethod = TrainParam::kUniform; @@ -137,7 +137,7 @@ TEST(GradientBasedSampler, GradientBasedSampling) { VerifySampling(kPageSize, kSubsample, kSamplingMethod); } -TEST(GradientBasedSampler, GradientBasedSampling_ExternalMemory) { +TEST(GradientBasedSampler, GradientBasedSamplingExternalMemory) { constexpr size_t kPageSize = 1024; constexpr float kSubsample = 0.8; constexpr int kSamplingMethod = TrainParam::kGradientBased; diff --git a/tests/cpp/tree/test_constraints.cu b/tests/cpp/tree/test_constraints.cu index 0bfe839ac..f1d73df8f 100644 --- a/tests/cpp/tree/test_constraints.cu +++ b/tests/cpp/tree/test_constraints.cu @@ -45,13 +45,13 @@ tree::TrainParam GetParameter() { } void CompareBitField(LBitField64 d_field, std::set positions) { - std::vector h_field_storage(d_field.bits_.size()); - thrust::copy(thrust::device_ptr(d_field.bits_.data()), + std::vector h_field_storage(d_field.Bits().size()); + thrust::copy(thrust::device_ptr(d_field.Bits().data()), thrust::device_ptr( - d_field.bits_.data() + d_field.bits_.size()), + d_field.Bits().data() + d_field.Bits().size()), h_field_storage.data()); - LBitField64 h_field; - h_field.bits_ = {h_field_storage.data(), h_field_storage.data() + h_field_storage.size()}; + LBitField64 h_field{ {h_field_storage.data(), + h_field_storage.data() + h_field_storage.size()} }; for (size_t i = 0; i < h_field.Size(); ++i) { if (positions.find(i) != positions.cend()) { @@ -73,13 +73,14 @@ TEST(GPUFeatureInteractionConstraint, Init) { ASSERT_EQ(constraints.Features(), kFeatures); common::Span s_nodes_constraints = constraints.GetNodeConstraints(); for (LBitField64 const& d_node : s_nodes_constraints) { - std::vector h_node_storage(d_node.bits_.size()); - thrust::copy(thrust::device_ptr(d_node.bits_.data()), - thrust::device_ptr( - d_node.bits_.data() + d_node.bits_.size()), + std::vector h_node_storage(d_node.Bits().size()); + thrust::copy(thrust::device_ptr(d_node.Bits().data()), + thrust::device_ptr( + d_node.Bits().data() + d_node.Bits().size()), h_node_storage.data()); - LBitField64 h_node; - h_node.bits_ = {h_node_storage.data(), h_node_storage.data() + h_node_storage.size()}; + LBitField64 h_node { + {h_node_storage.data(), h_node_storage.data() + h_node_storage.size()} + }; // no feature is attached to node. for (size_t i = 0; i < h_node.Size(); ++i) { ASSERT_FALSE(h_node.Check(i)); @@ -133,7 +134,7 @@ TEST(GPUFeatureInteractionConstraint, Split) { constraints.Split(0, /*feature_id=*/1, 1, 2); for (size_t nid = 0; nid < 3; ++nid) { d_node[nid] = constraints.GetNodeConstraints()[nid]; - ASSERT_EQ(d_node[nid].bits_.size(), 1); + ASSERT_EQ(d_node[nid].Bits().size(), 1); CompareBitField(d_node[nid], {1, 2}); } } diff --git a/tests/cpp/tree/test_gpu_hist.cu b/tests/cpp/tree/test_gpu_hist.cu index 9eca3e78d..930dc01c5 100644 --- a/tests/cpp/tree/test_gpu_hist.cu +++ b/tests/cpp/tree/test_gpu_hist.cu @@ -193,7 +193,7 @@ TEST(GpuHist, EvaluateSplits) { auto cmat = GetHostCutMatrix(); // Copy cut matrix to device. - page->cuts_ = cmat; + page->Cuts() = cmat; maker.ba.Allocate(0, &(maker.monotone_constraints), kNCols); dh::CopyVectorToDeviceSpan(maker.monotone_constraints, param.monotone_constraints); @@ -271,7 +271,7 @@ void TestHistogramIndexImpl() { const auto &maker_ext = hist_maker_ext.maker; std::vector h_gidx_buffer_ext(maker_ext->page->gidx_buffer.HostVector()); - ASSERT_EQ(maker->page->cuts_.TotalBins(), maker_ext->page->cuts_.TotalBins()); + ASSERT_EQ(maker->page->Cuts().TotalBins(), maker_ext->page->Cuts().TotalBins()); ASSERT_EQ(maker->page->gidx_buffer.Size(), maker_ext->page->gidx_buffer.Size()); } @@ -498,7 +498,7 @@ TEST(GpuHist, ExternalMemoryWithSampling) { } } -TEST(GpuHist, Config_IO) { +TEST(GpuHist, ConfigIO) { GenericParameter generic_param(CreateEmptyGenericParam(0)); std::unique_ptr updater {TreeUpdater::Create("grow_gpu_hist", &generic_param) }; updater->Configure(Args{}); diff --git a/tests/cpp/tree/test_quantile_hist.cc b/tests/cpp/tree/test_quantile_hist.cc index 39f2a3670..a0541ae78 100644 --- a/tests/cpp/tree/test_quantile_hist.cc +++ b/tests/cpp/tree/test_quantile_hist.cc @@ -73,7 +73,7 @@ class QuantileHistMock : public QuantileHistMaker { const size_t rid = batch.base_rowid + i; ASSERT_LT(rid, num_row); const size_t gmat_row_offset = gmat.row_ptr[rid]; - ASSERT_LT(gmat_row_offset, gmat.index.size()); + ASSERT_LT(gmat_row_offset, gmat.index.Size()); SparsePage::Inst inst = batch[i]; ASSERT_EQ(gmat.row_ptr[rid] + inst.size(), gmat.row_ptr[rid + 1]); for (size_t j = 0; j < inst.size(); ++j) { @@ -285,14 +285,14 @@ class QuantileHistMock : public QuantileHistMaker { } }; -TEST(Updater, QuantileHist_InitData) { +TEST(QuantileHist, InitData) { std::vector> cfg {{"num_feature", std::to_string(QuantileHistMock::GetNumColumns())}}; QuantileHistMock maker(cfg); maker.TestInitData(); } -TEST(Updater, QuantileHist_BuildHist) { +TEST(QuantileHist, BuildHist) { // Don't enable feature grouping std::vector> cfg {{"num_feature", std::to_string(QuantileHistMock::GetNumColumns())}, @@ -301,7 +301,7 @@ TEST(Updater, QuantileHist_BuildHist) { maker.TestBuildHist(); } -TEST(Updater, QuantileHist_EvalSplits) { +TEST(QuantileHist, EvalSplits) { std::vector> cfg {{"num_feature", std::to_string(QuantileHistMock::GetNumColumns())}, {"split_evaluator", "elastic_net"},