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

Add error for overflowing nbits during PQ construction #3833

Closed
wants to merge 1 commit into from

Conversation

mnorris11
Copy link

@mnorris11 mnorris11 commented Sep 4, 2024

Summary:
size_t expands to unsigned long int, so any left shift more than 31 means ksub overflows. So, we can add a check right before it is constructed in ProductQuantizer.

Ramil talked to Matthijs and the outcome is that 16 bits isn't really practical, but we can do the check for 24 to be safe.

Differential Revision: D62153881

@facebook-github-bot
Copy link
Contributor

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

…rch#3833)

Summary:
Pull Request resolved: facebookresearch#3833

size_t expands to unsigned long int, so any left shift more than 31 means ksub overflows. So, we can add a check right before it is constructed in ProductQuantizer.

Ramil talked to Matthijs and got to understand that over 16 bits isn't really practical, but we can do 24 to be safe.

Reviewed By: mengdilin

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

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

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in a4ebcb1.

aalekhpatel07 pushed a commit to aalekhpatel07/faiss that referenced this pull request Oct 17, 2024
…rch#3833)

Summary:
Pull Request resolved: facebookresearch#3833

size_t expands to unsigned long int, so any left shift more than 31 means ksub overflows. So, we can add a check right before it is constructed in ProductQuantizer.

Ramil talked to Matthijs and the outcome is that 16 bits isn't really practical, but we can do the check for 24 to be safe.

Reviewed By: kuarora, mengdilin

Differential Revision: D62153881

fbshipit-source-id: 721db6bf6ad5dd5d336b4498f4750acc4059310c
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.

2 participants