Skip to content

fix bug: IVFPQ of raft/cuvs does not require redundant check #4241

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.

Sign up for GitHub

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

Closed
weiwch wants to merge 5 commits into from
Closed

fix bug: IVFPQ of raft/cuvs does not require redundant check #4241

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
69f2a5c
fix: IVFPQ of raft/cuvs does not require pq length check for Faiss' o…
weiwch Mar 17, 2025
71b84e5
Merge branch 'main' into main
junjieqi Mar 20, 2025
20b25cd
Merge branch 'main' into main
junjieqi Mar 25, 2025
43fae0d
Merge branch 'main' into main
junjieqi Mar 31, 2025
538d565
refactor the fix
weiwch Apr 1, 2025
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
18 changes: 7 additions & 11 deletions faiss/gpu/GpuIndexIVFPQ.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -566,17 +566,13 @@ void GpuIndexIVFPQ::verifyPQSettings_() const {
"Bits per code must be 8 (passed %d)",
bitsPerCode_);
}
}

// The number of bytes per encoded vector must be one we support
FAISS_THROW_IF_NOT_FMT(
ivfpqConfig_.interleavedLayout ||
IVFPQ::isSupportedPQCodeLength(subQuantizers_),
"Number of bytes per encoded vector / sub-quantizers (%d) "
"is not supported",
subQuantizers_);

if (!should_use_cuvs(config_)) {
// The number of bytes per encoded vector must be one we support
Copy link
Contributor

@mnorris11 mnorris11 Apr 1, 2025

Choose a reason for hiding this comment

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

I think this whole block can be moved up into the block on line 557-567 right? now everything else in this function is !should_use_cuvs(config_)

Copy link
Contributor Author

@weiwch weiwch Apr 1, 2025

Choose a reason for hiding this comment

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

You are right! I have changed the code to the right place to make the logic of the code clearer.

FAISS_THROW_IF_NOT_FMT(
ivfpqConfig_.interleavedLayout ||
IVFPQ::isSupportedPQCodeLength(subQuantizers_),
"Number of bytes per encoded vector / sub-quantizers (%d) "
"is not supported",
subQuantizers_);
// Sub-quantizers must evenly divide dimensions available
FAISS_THROW_IF_NOT_FMT(
this->d % subQuantizers_ == 0,
Expand Down