Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions cpp/include/cuml/fil/detail/bitset.hpp

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is additional out-of-bound access for the unused bit-wise boolean operations. I will remove those in a follow-up.

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2023-2025, NVIDIA CORPORATION.
* SPDX-FileCopyrightText: Copyright (c) 2023-2026, NVIDIA CORPORATION.
* SPDX-License-Identifier: Apache-2.0
*/
#pragma once
Expand All @@ -22,6 +22,8 @@ struct bitset {
using storage_type = storage_t;
using index_type = index_t;

static_assert(std::is_unsigned_v<index_t>, "index_t must be unsigned");
Comment thread
chyunsu3 marked this conversation as resolved.
Resolved

auto constexpr static const bin_width = index_type(sizeof(storage_type) * 8);

HOST DEVICE bitset() : data_{nullptr}, num_bits_{0} {}
Expand All @@ -39,12 +41,12 @@ struct bitset {
// Standard bit-wise mutators and accessor
HOST DEVICE auto& set(index_type index)
{
data_[bin_from_index(index)] |= mask_in_bin(index);
if (index < num_bits_) { data_[bin_from_index(index)] |= mask_in_bin(index); }
Comment thread
chyunsu3 marked this conversation as resolved.
Resolved
return *this;
}
HOST DEVICE auto& clear(index_type index)
{
data_[bin_from_index(index)] &= ~mask_in_bin(index);
if (index < num_bits_) { data_[bin_from_index(index)] &= ~mask_in_bin(index); }
return *this;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
}
HOST DEVICE auto test(index_type index) const
Expand Down
37 changes: 27 additions & 10 deletions cpp/include/cuml/fil/detail/decision_forest_builder.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2023-2025, NVIDIA CORPORATION.
* SPDX-FileCopyrightText: Copyright (c) 2023-2026, NVIDIA CORPORATION.
Comment thread
chyunsu3 marked this conversation as resolved.
Resolved
* SPDX-License-Identifier: Apache-2.0
*/
#pragma once
Expand All @@ -18,6 +18,7 @@
#include <algorithm>
#include <cmath>
#include <cstddef>
#include <iterator>
#include <numeric>
#include <optional>
#include <vector>
Expand Down Expand Up @@ -57,20 +58,36 @@ struct decision_forest_builder {
typename node_type::metadata_storage_type feature = typename node_type::metadata_storage_type{},
typename node_type::offset_type offset = typename node_type::offset_type{})
{
auto constexpr const bin_width = index_type(sizeof(typename node_type::index_type) * 8);
auto node_value = typename node_type::index_type{};
auto set_storage = &node_value;
auto max_node_categories =
(vec_begin != vec_end) ? *std::max_element(vec_begin, vec_end) + 1 : 1;
auto constexpr const bin_width =
typename node_type::index_type{sizeof(typename node_type::index_type) * 8};
auto node_value = typename node_type::index_type{};
auto set_storage = &node_value;

// Ensure that all category indices are non-negative.
using cat_t = typename std::iterator_traits<iter_t>::value_type;
auto const is_negative = [](cat_t x) { return x < 0; };
if (std::any_of(vec_begin, vec_end, is_negative)) {
throw model_builder_error("Category index must be non-negative");
}

// Ensure that (max_cat + 1) can be represented as node_type::index_type
// to prevent integer overflow.
auto max_cat = (vec_begin != vec_end) ? *std::max_element(vec_begin, vec_end) : cat_t{0};
auto const max_representable = std::numeric_limits<typename node_type::index_type>::max();
if (max_cat >= max_representable || max_cat + 1 >= max_representable) {
throw model_builder_error(
"Category index exceeds maximum representable value for this model's index type");
}
Comment thread
chyunsu3 marked this conversation as resolved.
Resolved
auto max_cat_plus_one =
static_cast<typename node_type::index_type>(max_cat) + typename node_type::index_type{1};
if (max_num_categories_ > bin_width) {
// TODO(wphicks): Check for overflow here
node_value = categorical_storage_.size();
auto bins_required = raft_proto::ceildiv(max_node_categories, bin_width);
categorical_storage_.push_back(max_node_categories);
auto bins_required = raft_proto::ceildiv(max_cat_plus_one, bin_width);
categorical_storage_.push_back(max_cat_plus_one);
categorical_storage_.resize(categorical_storage_.size() + bins_required);
set_storage = &(categorical_storage_[node_value + 1]);
}
auto set = bitset{set_storage, max_node_categories};
auto set = bitset{set_storage, max_cat_plus_one};
std::for_each(vec_begin, vec_end, [&set](auto&& cat_index) { set.set(cat_index); });

add_node(
Expand Down
4 changes: 2 additions & 2 deletions cpp/include/cuml/fil/detail/raft_proto/ceildiv.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2023-2025, NVIDIA CORPORATION.
* SPDX-FileCopyrightText: Copyright (c) 2023-2026, NVIDIA CORPORATION.
* SPDX-License-Identifier: Apache-2.0
*/
#pragma once
Expand All @@ -9,6 +9,6 @@ namespace raft_proto {
template <typename T, typename U>
HOST DEVICE auto constexpr ceildiv(T dividend, U divisor)
{
return (dividend + divisor - T{1}) / divisor;
return dividend / divisor + (dividend % divisor != 0);
Comment thread
dantegd marked this conversation as resolved.
Resolved
}
} // namespace raft_proto
Loading