Skip to content

Use CCCL's mdspan implementation#1605

Merged
rapids-bot[bot] merged 20 commits intorapidsai:mainfrom
divyegala:cccl-mdspan
Dec 5, 2025
Merged

Use CCCL's mdspan implementation#1605
rapids-bot[bot] merged 20 commits intorapidsai:mainfrom
divyegala:cccl-mdspan

Conversation

@divyegala
Copy link
Copy Markdown
Member

@divyegala divyegala requested a review from a team as a code owner December 3, 2025 11:46
@divyegala divyegala requested a review from bdice December 3, 2025 11:46
Copy link
Copy Markdown
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Nice work!

Comment thread ci/build_cpp.sh Outdated
void extract_centers(raft::resources const& res,
const index<int64_t>& index,
raft::device_matrix_view<float, uint32_t, raft::row_major> cluster_centers);
raft::device_matrix_view<float, int64_t, raft::row_major> cluster_centers);
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.

We might need to change the signatures of ivf-pq getters to int64_t instead of uint32_t, right? For example things like list_sizes.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Can you share a link reference?

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.

yes so we have n_lists() for example

uint32_t n_lists() const noexcept;

Do we want to change this signature to int64_t?

Copy link
Copy Markdown
Contributor

@tarang-jain tarang-jain Dec 4, 2025

Choose a reason for hiding this comment

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

In other words, what I am trying to say is that we might be considering an index-wide migration to int64_t from uint32_t for all extent types in IVF-PQ.

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.

@tfeher had made of aware of plans for this, but maybe that is outside the scope of this PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

n_lists() returns lists_.size() and lists_ is an std::vector

std::vector<std::shared_ptr<list_data<IdxT>>> lists_;

It should be returning size_t. Anyway, the extent type updates in this PR are not related to index<IdxT> where IdxT is the index type, not the extent type.

This reverts commit 0f43b18.
@divyegala
Copy link
Copy Markdown
Member Author

/merge

@rapids-bot rapids-bot Bot merged commit 94795b0 into rapidsai:main Dec 5, 2025
93 checks passed
rapids-bot Bot pushed a commit to rapidsai/cuml that referenced this pull request Dec 5, 2025
@bdice bdice mentioned this pull request Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Introduces a breaking change feature request New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants