Skip to content

Commit

Permalink
Various small cleanups.
Browse files Browse the repository at this point in the history
- Add deduction guide for Span.
- Merge data split mode setter into the `MetaInfo` column sync method.
- Check CUDA warning for cross space function calls.
  • Loading branch information
trivialfis committed Dec 13, 2024
1 parent f4f3bd4 commit c3b4cc5
Show file tree
Hide file tree
Showing 14 changed files with 65 additions and 41 deletions.
9 changes: 8 additions & 1 deletion cmake/Utils.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ function(xgboost_set_cuda_flags target)
$<$<COMPILE_LANGUAGE:CUDA>:--expt-relaxed-constexpr>
$<$<COMPILE_LANGUAGE:CUDA>:-Xcompiler=${OpenMP_CXX_FLAGS}>
$<$<COMPILE_LANGUAGE:CUDA>:-Xfatbin=-compress-all>
$<$<COMPILE_LANGUAGE:CUDA>:--default-stream per-thread>)
$<$<COMPILE_LANGUAGE:CUDA>:--default-stream per-thread>
)

if(FORCE_COLORED_OUTPUT)
if(FORCE_COLORED_OUTPUT AND (CMAKE_GENERATOR STREQUAL "Ninja") AND
Expand Down Expand Up @@ -200,6 +201,12 @@ macro(xgboost_target_properties target)
if(WIN32 AND MINGW)
target_compile_options(${target} PUBLIC -static-libstdc++)
endif()

if (NOT WIN32 AND ENABLE_ALL_WARNINGS)
target_compile_options(${target} PRIVATE
$<$<COMPILE_LANGUAGE:CUDA>:-Werror=cross-execution-space-call>
)
endif()
endmacro()

# Custom definitions used in xgboost.
Expand Down
2 changes: 2 additions & 0 deletions include/xgboost/c_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -899,6 +899,8 @@ XGB_DLL int XGBoosterFree(BoosterHandle handle);
* @brief Reset the booster object to release data caches used for training.
*
* @since 3.0.0
*
* @return 0 when success, -1 when failure happens
*/
XGB_DLL int XGBoosterReset(BoosterHandle handle);

Expand Down
25 changes: 10 additions & 15 deletions include/xgboost/data.h
Original file line number Diff line number Diff line change
Expand Up @@ -172,31 +172,27 @@ class MetaInfo {
* columns.
*/
void Extend(MetaInfo const& that, bool accumulate_rows, bool check_column);

/**
* @brief Synchronize the number of columns across all workers.
*
* Normally we just need to find the maximum number of columns across all workers, but
* in vertical federated learning, since each worker loads its own list of columns,
* we need to sum them.
*/
void SynchronizeNumberOfColumns(Context const* ctx);

/*! \brief Whether the data is split row-wise. */
bool IsRowSplit() const {
return data_split_mode == DataSplitMode::kRow;
}
void SynchronizeNumberOfColumns(Context const* ctx, DataSplitMode split_mode);

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

/*!
* \brief A convenient method to check if we are doing vertical federated learning, which requires
/**
* @brief A convenient method to check if we are doing vertical federated learning, which requires
* some special processing.
*/
bool IsVerticalFederated() const;
[[nodiscard]] bool IsVerticalFederated() const;

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

Inst operator[](size_t i) const {
[[nodiscard]] Inst operator[](std::size_t i) const {
auto size = *(offset.data() + i + 1) - *(offset.data() + i);
return {data.data() + *(offset.data() + i),
static_cast<Inst::index_type>(size)};
return {data.data() + *(offset.data() + i), static_cast<Inst::index_type>(size)};
}

[[nodiscard]] size_t Size() const { return offset.size() == 0 ? 0 : offset.size() - 1; }
Expand Down
7 changes: 7 additions & 0 deletions include/xgboost/span.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include <limits> // numeric_limits
#include <type_traits>
#include <utility> // for move
#include <vector> // for vector

#if defined(__CUDACC__)
#include <cuda_runtime.h>
Expand Down Expand Up @@ -707,6 +708,12 @@ class IterSpan {
return it_ + size();
}
};

template <typename T>
Span(std::vector<T> const&) -> Span<T const>;

template <typename T>
Span(std::vector<T>&) -> Span<T>;
} // namespace xgboost::common


Expand Down
3 changes: 2 additions & 1 deletion include/xgboost/tree_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,8 @@ class RegTree : public Model {
return this->DefaultLeft() ? this->LeftChild() : this->RightChild();
}
/*! \brief feature index of split condition */
[[nodiscard]] XGBOOST_DEVICE unsigned SplitIndex() const {
[[nodiscard]] XGBOOST_DEVICE bst_feature_t SplitIndex() const {
static_assert(!std::is_signed_v<bst_feature_t>);
return sindex_ & ((1U << 31) - 1U);
}
/*! \brief when feature is unknown, whether goes to left child */
Expand Down
8 changes: 4 additions & 4 deletions src/common/host_device_vector.cu
Original file line number Diff line number Diff line change
Expand Up @@ -413,13 +413,13 @@ template class HostDeviceVector<bst_float>;
template class HostDeviceVector<double>;
template class HostDeviceVector<GradientPair>;
template class HostDeviceVector<GradientPairPrecise>;
template class HostDeviceVector<int32_t>; // bst_node_t
template class HostDeviceVector<uint8_t>;
template class HostDeviceVector<int8_t>;
template class HostDeviceVector<std::int32_t>; // bst_node_t
template class HostDeviceVector<std::uint8_t>;
template class HostDeviceVector<std::int8_t>;
template class HostDeviceVector<FeatureType>;
template class HostDeviceVector<Entry>;
template class HostDeviceVector<bst_idx_t>;
template class HostDeviceVector<uint32_t>; // bst_feature_t
template class HostDeviceVector<std::uint32_t>; // bst_feature_t
template class HostDeviceVector<RegTree::Node>;
template class HostDeviceVector<RegTree::CategoricalSplitMatrix::Segment>;
template class HostDeviceVector<RTreeNodeStat>;
Expand Down
9 changes: 9 additions & 0 deletions src/common/transform_iterator.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,15 @@ class IndexTransformIter {
++(*this);
return ret;
}
IndexTransformIter &operator--() {
iter_--;
return *this;
}
IndexTransformIter operator--(int) {
auto ret = *this;
--(*this);
return ret;
}
IndexTransformIter &operator+=(difference_type n) {
iter_ += n;
return *this;
Expand Down
3 changes: 2 additions & 1 deletion src/data/data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -770,7 +770,8 @@ void MetaInfo::Extend(MetaInfo const& that, bool accumulate_rows, bool check_col
}
}

void MetaInfo::SynchronizeNumberOfColumns(Context const* ctx) {
void MetaInfo::SynchronizeNumberOfColumns(Context const* ctx, DataSplitMode split_mode) {
this->data_split_mode = split_mode;
auto op = IsColumnSplit() ? collective::Op::kSum : collective::Op::kMax;
auto rc = collective::Allreduce(ctx, linalg::MakeVec(&num_col_, 1), op);
collective::SafeColl(rc);
Expand Down
1 change: 0 additions & 1 deletion src/data/iterative_dmatrix.cu
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ void IterativeDMatrix::InitFromCUDA(Context const* ctx, BatchParam const& p,
}

iter.Reset();
// Synchronise worker columns
}

IterativeDMatrix::IterativeDMatrix(std::shared_ptr<EllpackPage> ellpack, MetaInfo const& info,
Expand Down
2 changes: 1 addition & 1 deletion src/data/proxy_dmatrix.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ struct ExternalDataInfo {
info.num_row_ = this->accumulated_rows;
info.num_col_ = this->n_features;
info.num_nonzero_ = this->nnz;
info.SynchronizeNumberOfColumns(ctx);
info.SynchronizeNumberOfColumns(ctx, DataSplitMode::kRow);
this->Validate();
}
};
Expand Down
13 changes: 6 additions & 7 deletions src/data/simple_dmatrix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,10 @@ DMatrix* SimpleDMatrix::SliceCol(int num_slices, int slice_id) {
return out;
}

void SimpleDMatrix::ReindexFeatures(Context const* ctx) {
if (info_.IsColumnSplit() && collective::GetWorldSize() > 1) {
void SimpleDMatrix::ReindexFeatures(Context const* ctx, DataSplitMode split_mode) {
if (split_mode == DataSplitMode::kCol && collective::GetWorldSize() > 1) {
std::vector<std::uint64_t> buffer(collective::GetWorldSize());
buffer[collective::GetRank()] = info_.num_col_;
buffer[collective::GetRank()] = this->Info().num_col_;
auto rc = collective::Allgather(ctx, linalg::MakeVec(buffer.data(), buffer.size()));
SafeColl(rc);
auto offset = std::accumulate(buffer.cbegin(), buffer.cbegin() + collective::GetRank(), 0);
Expand Down Expand Up @@ -287,10 +287,9 @@ SimpleDMatrix::SimpleDMatrix(AdapterT* adapter, float missing, int nthread,
info_.num_col_ = adapter->NumColumns();
}

// Synchronise worker columns
info_.data_split_mode = data_split_mode;
ReindexFeatures(&ctx);
info_.SynchronizeNumberOfColumns(&ctx);
// Must called before sync column
this->ReindexFeatures(&ctx, data_split_mode);
this->Info().SynchronizeNumberOfColumns(&ctx, data_split_mode);

if (adapter->NumRows() == kAdapterUnknownSize) {
using IteratorAdapterT =
Expand Down
13 changes: 6 additions & 7 deletions src/data/simple_dmatrix.cu
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,14 @@ SimpleDMatrix::SimpleDMatrix(AdapterT* adapter, float missing, std::int32_t nthr
CopyToSparsePage(&ctx, adapter->Value(), device, missing, sparse_page_.get());
info_.num_col_ = adapter->NumColumns();
info_.num_row_ = adapter->NumRows();
// Synchronise worker columns
info_.data_split_mode = data_split_mode;
info_.SynchronizeNumberOfColumns(&ctx);

this->Info().SynchronizeNumberOfColumns(&ctx, data_split_mode);

this->fmat_ctx_ = ctx;
}

template SimpleDMatrix::SimpleDMatrix(CudfAdapter* adapter, float missing,
int nthread, DataSplitMode data_split_mode);
template SimpleDMatrix::SimpleDMatrix(CupyAdapter* adapter, float missing,
int nthread, DataSplitMode data_split_mode);
template SimpleDMatrix::SimpleDMatrix(CudfAdapter* adapter, float missing, std::int32_t nthread,
DataSplitMode data_split_mode);
template SimpleDMatrix::SimpleDMatrix(CupyAdapter* adapter, float missing, std::int32_t nthread,
DataSplitMode data_split_mode);
} // namespace xgboost::data
6 changes: 3 additions & 3 deletions src/data/simple_dmatrix.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class SimpleDMatrix : public DMatrix {
public:
SimpleDMatrix() = default;
template <typename AdapterT>
explicit SimpleDMatrix(AdapterT* adapter, float missing, int nthread,
explicit SimpleDMatrix(AdapterT* adapter, float missing, std::int32_t nthread,
DataSplitMode data_split_mode = DataSplitMode::kRow);

explicit SimpleDMatrix(dmlc::Stream* in_stream);
Expand Down Expand Up @@ -61,14 +61,14 @@ class SimpleDMatrix : public DMatrix {
bool SparsePageExists() const override { return true; }

/**
* \brief Reindex the features based on a global view.
* @brief Reindex the features based on a global view.
*
* In some cases (e.g. column-wise data split and vertical federated learning), features are
* loaded locally with indices starting from 0. However, all the algorithms assume the features
* are globally indexed, so we reindex the features based on the offset needed to obtain the
* global view.
*/
void ReindexFeatures(Context const* ctx);
void ReindexFeatures(Context const* ctx, DataSplitMode split_mode);

private:
// Context used only for DMatrix initialization.
Expand Down
5 changes: 5 additions & 0 deletions tests/cpp/common/test_span.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ TEST(Span, TestStatus) {
int status = 1;
TestTestStatus {&status}();
ASSERT_EQ(status, -1);

std::vector<double> foo;
auto bar = Span{foo};
ASSERT_FALSE(bar.data());
ASSERT_EQ(bar.size(), 0);
}

TEST(Span, DlfConstructors) {
Expand Down

0 comments on commit c3b4cc5

Please sign in to comment.