Skip to content

Commit eea2186

Browse files
authored
Various small cleanups. (dmlc#11097)
- Add deduction guide for Span. - Merge data split mode setter into the `MetaInfo` column sync method. - Check CUDA warning for cross space function calls.
1 parent ef29f0b commit eea2186

File tree

14 files changed

+66
-42
lines changed

14 files changed

+66
-42
lines changed

cmake/Utils.cmake

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,8 @@ function(xgboost_set_cuda_flags target)
8181
$<$<COMPILE_LANGUAGE:CUDA>:--expt-relaxed-constexpr>
8282
$<$<COMPILE_LANGUAGE:CUDA>:-Xcompiler=${OpenMP_CXX_FLAGS}>
8383
$<$<COMPILE_LANGUAGE:CUDA>:-Xfatbin=-compress-all>
84-
$<$<COMPILE_LANGUAGE:CUDA>:--default-stream per-thread>)
84+
$<$<COMPILE_LANGUAGE:CUDA>:--default-stream per-thread>
85+
)
8586

8687
if(FORCE_COLORED_OUTPUT)
8788
if(FORCE_COLORED_OUTPUT AND (CMAKE_GENERATOR STREQUAL "Ninja") AND
@@ -200,6 +201,12 @@ macro(xgboost_target_properties target)
200201
if(WIN32 AND MINGW)
201202
target_compile_options(${target} PUBLIC -static-libstdc++)
202203
endif()
204+
205+
if(NOT WIN32 AND ENABLE_ALL_WARNINGS)
206+
target_compile_options(${target} PRIVATE
207+
$<$<COMPILE_LANGUAGE:CUDA>:-Werror=cross-execution-space-call>
208+
)
209+
endif()
203210
endmacro()
204211

205212
# Custom definitions used in xgboost.

include/xgboost/c_api.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -899,6 +899,8 @@ XGB_DLL int XGBoosterFree(BoosterHandle handle);
899899
* @brief Reset the booster object to release data caches used for training.
900900
*
901901
* @since 3.0.0
902+
*
903+
* @return 0 when success, -1 when failure happens
902904
*/
903905
XGB_DLL int XGBoosterReset(BoosterHandle handle);
904906

include/xgboost/data.h

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -172,31 +172,27 @@ class MetaInfo {
172172
* columns.
173173
*/
174174
void Extend(MetaInfo const& that, bool accumulate_rows, bool check_column);
175-
176175
/**
177176
* @brief Synchronize the number of columns across all workers.
178177
*
179178
* Normally we just need to find the maximum number of columns across all workers, but
180179
* in vertical federated learning, since each worker loads its own list of columns,
181180
* we need to sum them.
182181
*/
183-
void SynchronizeNumberOfColumns(Context const* ctx);
184-
185-
/*! \brief Whether the data is split row-wise. */
186-
bool IsRowSplit() const {
187-
return data_split_mode == DataSplitMode::kRow;
188-
}
182+
void SynchronizeNumberOfColumns(Context const* ctx, DataSplitMode split_mode);
189183

184+
/** @brief Whether the data is split row-wise. */
185+
[[nodiscard]] bool IsRowSplit() const { return data_split_mode == DataSplitMode::kRow; }
190186
/** @brief Whether the data is split column-wise. */
191-
bool IsColumnSplit() const { return data_split_mode == DataSplitMode::kCol; }
187+
[[nodiscard]] bool IsColumnSplit() const { return data_split_mode == DataSplitMode::kCol; }
192188
/** @brief Whether this is a learning to rank data. */
193-
bool IsRanking() const { return !group_ptr_.empty(); }
189+
[[nodiscard]] bool IsRanking() const { return !group_ptr_.empty(); }
194190

195-
/*!
196-
* \brief A convenient method to check if we are doing vertical federated learning, which requires
191+
/**
192+
* @brief A convenient method to check if we are doing vertical federated learning, which requires
197193
* some special processing.
198194
*/
199-
bool IsVerticalFederated() const;
195+
[[nodiscard]] bool IsVerticalFederated() const;
200196

201197
/*!
202198
* \brief A convenient method to check if the MetaInfo should contain labels.
@@ -330,10 +326,9 @@ struct HostSparsePageView {
330326
common::Span<bst_idx_t const> offset;
331327
common::Span<Entry const> data;
332328

333-
Inst operator[](size_t i) const {
329+
[[nodiscard]] Inst operator[](std::size_t i) const {
334330
auto size = *(offset.data() + i + 1) - *(offset.data() + i);
335-
return {data.data() + *(offset.data() + i),
336-
static_cast<Inst::index_type>(size)};
331+
return {data.data() + *(offset.data() + i), static_cast<Inst::index_type>(size)};
337332
}
338333

339334
[[nodiscard]] size_t Size() const { return offset.size() == 0 ? 0 : offset.size() - 1; }

include/xgboost/span.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
#include <limits> // numeric_limits
3838
#include <type_traits>
3939
#include <utility> // for move
40+
#include <vector> // for vector
4041

4142
#if defined(__CUDACC__)
4243
#include <cuda_runtime.h>
@@ -707,6 +708,12 @@ class IterSpan {
707708
return it_ + size();
708709
}
709710
};
711+
712+
template <typename T>
713+
Span(std::vector<T> const&) -> Span<T const>;
714+
715+
template <typename T>
716+
Span(std::vector<T>&) -> Span<T>;
710717
} // namespace xgboost::common
711718

712719

include/xgboost/tree_model.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,8 @@ class RegTree : public Model {
186186
return this->DefaultLeft() ? this->LeftChild() : this->RightChild();
187187
}
188188
/*! \brief feature index of split condition */
189-
[[nodiscard]] XGBOOST_DEVICE unsigned SplitIndex() const {
189+
[[nodiscard]] XGBOOST_DEVICE bst_feature_t SplitIndex() const {
190+
static_assert(!std::is_signed_v<bst_feature_t>);
190191
return sindex_ & ((1U << 31) - 1U);
191192
}
192193
/*! \brief when feature is unknown, whether goes to left child */

src/common/host_device_vector.cu

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -413,13 +413,13 @@ template class HostDeviceVector<bst_float>;
413413
template class HostDeviceVector<double>;
414414
template class HostDeviceVector<GradientPair>;
415415
template class HostDeviceVector<GradientPairPrecise>;
416-
template class HostDeviceVector<int32_t>; // bst_node_t
417-
template class HostDeviceVector<uint8_t>;
418-
template class HostDeviceVector<int8_t>;
416+
template class HostDeviceVector<std::int32_t>; // bst_node_t
417+
template class HostDeviceVector<std::uint8_t>;
418+
template class HostDeviceVector<std::int8_t>;
419419
template class HostDeviceVector<FeatureType>;
420420
template class HostDeviceVector<Entry>;
421421
template class HostDeviceVector<bst_idx_t>;
422-
template class HostDeviceVector<uint32_t>; // bst_feature_t
422+
template class HostDeviceVector<std::uint32_t>; // bst_feature_t
423423
template class HostDeviceVector<RegTree::Node>;
424424
template class HostDeviceVector<RegTree::CategoricalSplitMatrix::Segment>;
425425
template class HostDeviceVector<RTreeNodeStat>;

src/common/transform_iterator.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,15 @@ class IndexTransformIter {
6262
++(*this);
6363
return ret;
6464
}
65+
IndexTransformIter &operator--() {
66+
iter_--;
67+
return *this;
68+
}
69+
IndexTransformIter operator--(int) {
70+
auto ret = *this;
71+
--(*this);
72+
return ret;
73+
}
6574
IndexTransformIter &operator+=(difference_type n) {
6675
iter_ += n;
6776
return *this;

src/data/data.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -770,7 +770,8 @@ void MetaInfo::Extend(MetaInfo const& that, bool accumulate_rows, bool check_col
770770
}
771771
}
772772

773-
void MetaInfo::SynchronizeNumberOfColumns(Context const* ctx) {
773+
void MetaInfo::SynchronizeNumberOfColumns(Context const* ctx, DataSplitMode split_mode) {
774+
this->data_split_mode = split_mode;
774775
auto op = IsColumnSplit() ? collective::Op::kSum : collective::Op::kMax;
775776
auto rc = collective::Allreduce(ctx, linalg::MakeVec(&num_col_, 1), op);
776777
collective::SafeColl(rc);

src/data/iterative_dmatrix.cu

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,6 @@ void IterativeDMatrix::InitFromCUDA(Context const* ctx, BatchParam const& p,
100100
}
101101

102102
iter.Reset();
103-
// Synchronise worker columns
104103
}
105104

106105
IterativeDMatrix::IterativeDMatrix(std::shared_ptr<EllpackPage> ellpack, MetaInfo const& info,

src/data/proxy_dmatrix.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ struct ExternalDataInfo {
173173
info.num_row_ = this->accumulated_rows;
174174
info.num_col_ = this->n_features;
175175
info.num_nonzero_ = this->nnz;
176-
info.SynchronizeNumberOfColumns(ctx);
176+
info.SynchronizeNumberOfColumns(ctx, DataSplitMode::kRow);
177177
this->Validate();
178178
}
179179
};

0 commit comments

Comments
 (0)