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

Fix IVFPQFastScan decode function #3312

Closed
wants to merge 1 commit into from
Closed

Fix IVFPQFastScan decode function #3312

wants to merge 1 commit into from

Conversation

junjieqi
Copy link
Contributor

Summary:
as the #issue3258 mentioned, the IVFPQFastScan should have same decoding result as IVFPQ. However, current result is not as expected.

In this PR/Diff, we are going to fix the decoding function

Differential Revision: D55264781

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55264781

Summary:

as the [#issue3258](#3258) mentioned, the IVFPQFastScan should have same decoding result as IVFPQ. However, current result is not as expected.

In this PR/Diff, we are going to fix the decoding function

Differential Revision: D55264781
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D55264781

pq.decode(bytes, x, n);
size_t coarse_size = coarse_code_size();

#pragma omp parallel
Copy link
Contributor

@alexanderguzhva alexanderguzhva Mar 22, 2024

Choose a reason for hiding this comment

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

parallel if (n > 1)
or, even better, if n is greater than a certain threshold
please check other sa_decode implementations for the appropriate threshold

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @alexanderguzhva, thanks for the suggestion. I checked other sa_decode. They also don't have the condition. Try to understand it more. #pragma omp parallel if (n > 1) means we only parallel execution when n is greater than 1. I'm wondering how we should choose the threshold in general. Do we have any benchmark to run or other methods?

Copy link
Contributor

Choose a reason for hiding this comment

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

no benchmarks, this is a magic constant. Typically, things like 100 or 1000 are used.

#pragma omp parallel if (n > 1000)

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 14b8af6.

@junjieqi junjieqi deleted the export-D55264781 branch March 25, 2024 19:00
abhinavdangeti pushed a commit to blevesearch/faiss that referenced this pull request Jul 12, 2024
Summary:
Pull Request resolved: facebookresearch#3312

as the [#issue3258](facebookresearch#3258) mentioned, the IVFPQFastScan should have same decoding result as IVFPQ. However, current result is not as expected.

In this PR/Diff, we are going to fix the decoding function

Reviewed By: mdouze

Differential Revision: D55264781

fbshipit-source-id: dfdae9eabceadfc5a3ebb851930d71ce3c1c654d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants