MB-62985 - Add functionality to support binary quantised vectors. #42
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
metonymic-smokey
force-pushed
the
bq
branch
2 times, most recently
from
0299389 to
4132f92
Compare
metonymic-smokey
requested review from
CascadingRadium,
Likith101,
Thejas-bhat and
abhinavdangeti
metonymic-smokey
changed the title
|
|
||
| go 1.21 | ||
| go 1.22 | ||
|
|
Member
Author
There was a problem hiding this comment.
Was changed in a recent patch so update.
This reverts commit 73e9e4b.
index.go
Outdated
| return int(C.faiss_IndexBinary_d(idx.idxBinary)) | ||
| } | ||
|
|
||
| func (idx *faissIndex) IsTrained() bool { |
Member
There was a problem hiding this comment.
If index is binary then calling this method would panic right as you should be calling
return C.faiss_IndexBinary_is_trained(idx.idx) != 0
index.go
Outdated
| return | ||
| } | ||
|
|
||
| func (idx *faissIndex) SearchBinaryWithIDs(x []uint8, k int64, |
Member
There was a problem hiding this comment.
- please add all binary index related methods(and the struct) to a new file, after splitting the interface thanks
- The method naming is off, youre just searching the binary vector with a given K - No IDs for an include selector is given, Rename to Just SearchBinary
index.go
Outdated
| } | ||
| defer searchParams.Delete() | ||
|
|
||
| if c := C.faiss_IndexBinary_search_with_params( |
Member
There was a problem hiding this comment.
code block can be reused. Add searchBinaryWithParams method similar to searchWithParams
| tempBuf := (*C.uchar)(nil) | ||
| bufSize := C.size_t(0) | ||
|
|
||
| if c := C.faiss_write_index_binary_buf( |
Member
There was a problem hiding this comment.
Lot of code duplication here; following my earlier comment of split interfaces, you can just reuse the existing method
func WriteIndexIntoBuffer(idx BaseIndex) ([]byte, error) {
// the values to be returned by the faiss APIs
tempBuf := (*C.uchar)(nil)
bufSize := C.size_t(0)
switch i := idx.(type) {
case BinaryIndex: // Child Interface check - Will be binary index struct if has the TrainBinary method
status = C.faiss_write_index_binary_buf(
(*C.FaissIndexBinary)(i.(*binaryIndex).idx),
&bufSize,
&tempBuf,
)
case FloatIndex: // Child Interface check - Will be float index struct if has the IVF methods (example
status = C.faiss_write_index_buf(
(*C.FaissIndex)(i.(*floatIndex).idx),
&bufSize,
&tempBuf,
)
default:
return nil, fmt.Errorf("unsupported index type: %T", idx)
}
// Rest of the method unchanged
index_io.go
Outdated
| return &IndexImpl{&idx}, nil | ||
| } | ||
|
|
||
| func ReadBinaryIndexFromBuffer(buf []byte, ioflags int) (*IndexImpl, error) { |
Member
There was a problem hiding this comment.
Do not need a new method, just pass a flag, maybe expose a enum to zapx that can
type IndexType int
const (
FloatIndexType IndexType = iota // 0
BinaryIndexType // 1
)
func ReadIndexFromBuffer(buf []byte, ioflags int, idxType IndexType) () {
``
a961c59 to
5dd343f
Compare
abhinavdangeti
requested changes
Jun 6, 2025
metonymic-smokey
force-pushed
the
bq
branch
2 times, most recently
from
b779132 to
2c060eb
Compare
metonymic-smokey
force-pushed
the
bq
branch
2 times, most recently
from
1226060 to
20b3ae4
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.