Closed HammadB closed 1 week ago
Please leverage this checklist to ensure your code review is thorough before approving
This PR is not ready for review. I wouldn’t waste your time - it’s a draft. I’ll mark it when it’s ready.
Hammad Bashir EECS | Cal
On Sat, Jun 15, 2024 at 7:16 PM Sanket Kedia @.***> wrote:
@.**** commented on this pull request.
In rust/worker/src/execution/operators/brute_force_knn.rs https://github.com/chroma-core/chroma/pull/2347#discussion_r1641595435:
if !input.allowed_ids.is_empty()
&& !input .allowed_ids_brute_force
- .contains(&log_record.record.id)
- .contains(&log_record.merged_user_id())
There is a merged_user_id_ref() that does not clone() that can be used here probably?
— Reply to this email directly, view it on GitHub https://github.com/chroma-core/chroma/pull/2347#pullrequestreview-2121070927, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKW32LACTRZ5NVKZVGMXVTZHTYPVAVCNFSM6AAAAABJLQ2PM2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCMRRGA3TAOJSG4 . You are receiving this because you authored the thread.Message ID: @.***>
This is now ready for prelim review. I have my own cleanup and test cleanup to do but an eager eye is welcome to look.
Description of changes
This makes the bf knn operator use materialized logs. This makes the operator valid for cases like an invalid add etc, since those are filtered out of the log.
Tasks
This is a WIP (BUT Is now ready for review minus tests, will add those shortly)ideally in separate operator