-
Notifications
You must be signed in to change notification settings - Fork 682
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
Enabling BQ compression through update user config #3875
Conversation
…ons/setup-go-5 Bump actions/setup-go from 4 to 5
…factor-grpc-search-reply-parsing
…sing Introduce custom `pb.Properties` message to contain type-aware properties within search result
[flat] Fix rescoreLimit update
name: "setting bq compression on", | ||
initial: ent.UserConfig{ | ||
BQ: ent.BQConfig{ | ||
Enabled: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be false
initially?
if err := h.commitLog.AddPQ(h.compressor.ExposeFields()); err != nil { | ||
return errors.Wrap(err, "Adding PQ to the commit logger") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no longer needed? if so ExposeFields()
could be removed from VectorCompressor
interface as this seems to be only usage
h.shardedNodeLocks.RLock(0) | ||
node := h.nodes[0] | ||
h.shardedNodeLocks.RUnlock(0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you please merge master into HNSW_BQ
branch. I believe is changed already
cleanData := make([][]float32, 0, len(data)) | ||
for _, point := range data { | ||
if point == nil { | ||
continue | ||
} | ||
cleanData = append(cleanData, point) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I understand index of vector in cache is vector's id
. Here all empty element are removed, but as a result indexes change. Is this change safe? Are we sure that indexes are not lost or mixed later on?
(I see that compressor's cache is grown to size of cleaned up slice, but that may be to small for highest ids if some empty elements were indeed removed)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cleanData
is only used for fitting KMeans
. Latter, the points are added using data
instead of cleanData
and this is when they are added to the cache. The two slices are needed since KMeans
does expect a nil free array so the code is safe since the id
is only needed for the cache where we do use data
and not cleanData
.
Fixes for GRPC filters
Secure access to consistency level during batch deletes
Add support for geo coordinates in GRCP
|
What's being changed:
Enabling BQ compression through update user config
Review checklist