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

Remove static checks for serialization size #1997

Merged

Conversation

cjnolet
Copy link
Member

@cjnolet cjnolet commented Nov 15, 2023

This PR removes static checks for serialization size. Upstream changes like rapidsai/rmm#1370 have altered these sizes and break RAFT CI. An alternative approach to verifying serialization will be developed.

@cjnolet cjnolet added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Nov 15, 2023
@cjnolet cjnolet requested a review from a team as a code owner November 15, 2023 16:35
@cjnolet cjnolet self-assigned this Nov 15, 2023
@github-actions github-actions bot added the cpp label Nov 15, 2023
@bdice
Copy link
Contributor

bdice commented Nov 15, 2023

There are more places to fix in cagra_serialize.cuh:

  /home/coder/raft/cpp/include/raft/neighbors/detail/cagra/cagra_serialize.cuh(40): error: static assertion failed with "The size of the index struct has changed since the last update; paste in the new size and consider updating the serialization logic"
            detected during instantiation of class "raft::neighbors::cagra::detail::check_index_layout<RealSize, ExpectedSize> [with RealSize=216UL, ExpectedSize=200UL]" 

Also I believe the serialization version is expected to increment every time this changes. See #1964 for an example of the changes we'll need.

@bdice bdice changed the title Updating ivf-flat serialization size Remove static checks for serialization size Nov 15, 2023
@cjnolet
Copy link
Member Author

cjnolet commented Nov 15, 2023

/merge

Copy link
Member

@harrism harrism left a comment

Choose a reason for hiding this comment

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

Thanks for removing these checks. Will make life easier upstream.

@rapids-bot rapids-bot bot merged commit 31fcbf1 into rapidsai:branch-23.12 Nov 15, 2023
61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpp improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
Development

Successfully merging this pull request may close these issues.

4 participants