Automatic adjustment of itopk size according to filtering rate #509
Conversation
…when the number of results is large Fix some issues Fix lower recall issue with new multi-cta algo Removing redundant code and changing some parameters Update cpp/src/neighbors/detail/cagra/search_plan.cuh Co-authored-by: Tamas Bela Feher <[email protected]> Remove an unnecessary line and satisfy clang-format
anaruse
force-pushed
the
adjust_itopk
branch
2 times, most recently
from
d2e8d4c to
aa7cfde
Compare
cjnolet
added
improvement
|
/ok to test |
Handle the case when the search result contains invalid indices when building the updated graph in add_nodes. For debugging purposes, fail if any invalid indices found; in future, we can replace RAFT_FAIL with RAFT_LOG_WARN to make the add_nodes routine more robust.
Co-authored-by: Artem M. Chirkin <[email protected]>
| * The value must be equal to or greater than 0.0 and less than 1.0. Default value is | ||
| * negative, in which case the filtering rate is automatically set. | ||
| */ | ||
| float filtering_rate = -1.0; |
There was a problem hiding this comment.
This is interesting. So is specifying this value an alternative to explicitly computing the sparsity of the pre-filtering bitset (which would introduce additional compute into the search)?
There was a problem hiding this comment.
It would be clearer to write "automatically computed" rather than "automatically set" here. I will fix it.
There was a problem hiding this comment.
I like that. "automatically compted" is perfect.
| @@ -349,9 +351,18 @@ void search(raft::resources const& res, | |||
| auto& sample_filter = | |||
| dynamic_cast<const cuvs::neighbors::filtering::bitset_filter<uint32_t, int64_t>&>( | |||
| sample_filter_ref); | |||
| search_params params_copy = params; | |||
| if (params.filtering_rate < 0.0) { | |||
There was a problem hiding this comment.
Oh I see- the filtering rate set to "auto" will automatically calculate the sparsity of the bitset while a user can skip having to compute this if they set the filtering rate directly. I like this.
There was a problem hiding this comment.
That is correct. By default, it calculates the filtering rate, but if the user specifies the filtering rate, that calculation is skipped.
|
/ok to test |
|
/ok to test |
|
/ok to test |
|
@anaruse I think the changes look great. Can you also add the new search param to the main indexing docs for completeness? That would be the file in |
I've updated text in the filtering considerations section of |
|
/ok to test |
|
/ok to test |
|
/merge |
This PR is based on #492.
The new multi-CTA algorithm proposed in #492 can be used to obtain good recall even with high filtering rates. However, good recall cannot be obtained unless the number of search iterations, or itopk size, one of CAGRA's search parameters, is appropriately increased according to the filtering rate. Therefore, users need to find the appropriate itopk size according to the filtering rate by trial and error, which is a pain.
This PR is intended to alleviate this problem by internally calculating the filtering rate and automatically adjusting the itopk size accordingly.