Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feat] add repeat, sparsity, eval_n_elements APIs to bitset #2439

Open
wants to merge 1 commit into
base: branch-24.10
Choose a base branch
from

Conversation

rhdong
Copy link
Member

@rhdong rhdong commented Sep 18, 2024

  • This PR is a part of the feature that applies the prefilter brute-force in Cagra.

@rhdong rhdong requested a review from lowener September 18, 2024 21:05
@rhdong rhdong requested a review from a team as a code owner September 18, 2024 21:05
@github-actions github-actions bot added the cpp label Sep 18, 2024
@rhdong rhdong added enhancement New feature or request 3 - Ready for Review feature request New feature or request non-breaking Non-breaking change and removed cpp labels Sep 18, 2024
auto nnz_view = raft::make_device_scalar_view<index_t>(nnz.data());
auto size_view = raft::make_host_scalar_view<index_t>(&size_h);

raft::popc(res, vector_view, size_view, nnz_view);
Copy link
Contributor

Choose a reason for hiding this comment

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

The count() method can also be implemented in bitset_view, it will be useful for users.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am also noticing that if the number of bits is not a multiple of the bitset element size the result might be wrong.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am also noticing that if the number of bits is not a multiple of the bitset element size the result might be wrong.

Should not, because the size is the actual number of bits. Maybe I miss something?

@@ -145,6 +194,37 @@ class BitsetTest : public testing::TestWithParam<test_spec_bitset> {
resource::sync_stream(res, stream);
ASSERT_TRUE(hostVecMatch(bitset_ref, bitset_result, raft::Compare<bitset_t>()));

// test sparsity, repeat and eval_n_elements
if constexpr (std::is_same_v<bitset_t, uint32_t> || std::is_same_v<bitset_t, uint64_t>) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why restrict the test to those data types?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review enhancement New feature or request feature request New feature or request non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants