[Java] Adding tests to use CuVSDeviceMatrix (device memory) directly as a CagraIndex input dataset#1340
Conversation
| public void close() { | ||
| if (mustClose()) { | ||
| checkCuVSError(cuvsRMMFree(cuvsResourceHandle, pointer, numBytes), "cuvsRMMFree"); | ||
| pointer = MemorySegment.NULL; |
There was a problem hiding this comment.
This is a small enhancement that came up while testing: it protects us from a double close().
I thought it was worth adding it.
|
|
||
| for (int n = 0; n < rows; ++n) { | ||
| for (int i = 0; i < cols; ++i) { | ||
| assertEquals(array1[n][i], array2[n][i], DELTA); |
There was a problem hiding this comment.
This is the sort of thing that C++ does nicely. :]
assertSame2dArray would be a function template. After the element type T of array1 is deduced, line#265 would be a call to compareEquals<T>, which would have type-specific specializations.
compareEquals<float> would use DELTA for the epsilon-compare.
There was a problem hiding this comment.
All that said, this changed version does remove quite a bit of code repetition. 👏
There was a problem hiding this comment.
This is the sort of thing that C++ does nicely. :]
Yep.
I have planned another iteration of test cleanup/extension, as I have spotted other places where we are uncovered/where we can improve things.
Let me think if I can further de-duplicate here.
There was a problem hiding this comment.
Let me think if I can further de-duplicate here.
I don't think that's necessary. It's pretty sleek right now. We can add polish later, if required.
mythrocks
added
improvement
|
/ok to test 8c80760 |
|
/ok to test 00f10f6 |
mythrocks
changed the title
|
/ok to test 841b107 |
|
/merge |
github-project-automation
Bot
moved this from Todo
to Done
in Unstructured Data Processing
|
This change has been merged. Thank you for the contribution, @ldematte. |
…as a CagraIndex input dataset (rapidsai#1340) When we introduced `CuVSDeviceMatrix` in rapidsai#1232, we made it possible to use device-memory-backed dataset as an input for index build: since we accept a `CuVSMatrix`, and we have correct `toTensor` implementations for CPU and GPU, and the underlying functions in libcuvs support different memory types and sizes (through DLManagedTensor information), this became supported "naturally". However, we never tested this explicitly. This PR adds tests to check and show that using CuVSDeviceMatrix (device memory) directly as a CagraIndex input dataset works as intended. (similar tests for other index types will be added as follow-ups) Authors: - Lorenzo Dematté (https://github.com/ldematte) - MithunR (https://github.com/mythrocks) Approvers: - Chris Hegarty (https://github.com/ChrisHegarty) - MithunR (https://github.com/mythrocks) URL: rapidsai#1340
3 participants
When we introduced
CuVSDeviceMatrixin #1232, we made it possible to use device-memory-backed dataset as an input for index build: since we accept aCuVSMatrix, and we have correcttoTensorimplementations for CPU and GPU, and the underlying functions in libcuvs support different memory types and sizes (through DLManagedTensor information), this became supported "naturally".However, we never tested this explicitly.
This PR adds tests to check and show that using CuVSDeviceMatrix (device memory) directly as a CagraIndex input dataset works as intended.
(similar tests for other index types will be added as follow-ups)