Java: Fixing the broken pre-filtering for Brute Force #706
Conversation
|
relates #697 |
java/internal/src/cuvs_java.c
Outdated
| int64_t prefilter_shape[1] = {(n_queries * n_rows + 31) / 32}; | ||
| DLManagedTensor prefilter_tensor = prepare_tensor(prefilter_data_d, prefilter_shape, kDLUInt, 32, 1, kDLCUDA); | ||
| // Parse the filters data | ||
| uint32_t *prefilters = (uint32_t*) malloc(prefilter_data_length); |
There was a problem hiding this comment.
I don't understand why it is necessary to malloc and copy the pre filter data. Is it not possible to access it directly from the passed pointer?
There was a problem hiding this comment.
I had to change the byte ordering to make it work. Just type casting it to uint32_t * didn't work. I'll explore if doing some in-place transformation without allocating a new array is feasible.
There was a problem hiding this comment.
It should be possible to write the expected byte order at the java level,
There was a problem hiding this comment.
On the C++ side I added last month an option to specify the byte order used at the time of the creation of the prefilter data: https://github.com/rapidsai/raft/blob/branch-25.04/cpp/include/raft/core/bitmap.hpp#L64.
I think in your case it was created with uint8_t? So specifying original_nbits = sizeof(uint8_t) should prevent you from that extra allocation/copy. It is not present yet in the C API though.
There was a problem hiding this comment.
@ChrisHegarty fixed, it was unnecessary to change the order. I was doing things wrong. Fixed now.
cjnolet
added
bug
|
/ok to test |
|
Hi @chatman , the style checking seems to fail, may you have time to take a look? thanks! |
|
FYI, for larger dataset sizes, this fix is incorrect. Working on the proper fix. |
|
/ok to test |
1 similar comment
|
/ok to test |
chatman
force-pushed
the
ishan/prefiltering-fix
branch
from
a53dbf2 to
b6a7661
Compare
|
I think this is ready for merge. Please review @rhdong @ChrisHegarty @narangvivek10. |
|
/ok to test |
|
@rhdong Upon testing, I'm finding that on some systems, this is not working. On such systems, search call is returning non success return value, with invalid memory access. I copied the prefilter data into a device array and passed that array, and it works again. Wondering why it is working on one system with just the host array, but not on others. I'll continue the investigation and fix this by tomorrow (hopefully, before the freeze for the next release). FYI @cjnolet. |
Hi @chatman , the API always uses the filter memory pointer as a device accessible one, a little weird that |
Changed the signature from
long[] prefiltertoBitSet[] prefilters.Added a very simple test.