Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[BUG] TiledKNNTest fails #1526

Closed
enp1s0 opened this issue May 18, 2023 · 3 comments · Fixed by #1533
Closed

[BUG] TiledKNNTest fails #1526

enp1s0 opened this issue May 18, 2023 · 3 comments · Fixed by #1533
Assignees
@benfred
Labels
bug Something isn't working

Comments

@enp1s0
Copy link
Member

enp1s0 commented May 18, 2023 •
edited
Loading

There is an issue with the TiledKNNTest in the latest branch-23.06 (commit 8e412b4), as it fails. However, it works properly when reverting 6fdb041 .

@benfred, can you provide any insight into what might be causing this issue?

test log

[ RUN      ] TiledKNNTest/TiledKNNTestF.BruteForce/15
/opt/conda/conda-bld/work/cpp/test/neighbors/tiled_knn.cu:174: Failure
Value of: raft::spatial::knn::devArrMatchKnnPair(ref_indices_.data(), raft_indices_.data(), ref_distances_.data(), raft_distances_.data(), num_queries, k_, float(0.001), stream_)
  Actual: false (actual=19258,4.8802375793457031!=expected19829,4.8662505149841309 @0,0)
Expected: true

ref: https://github.com/rapidsai/raft/actions/runs/5010580599/jobs/8981566095?pr=1514
@enp1s0 enp1s0 added the bug Something isn't working label May 18, 2023
@benfred benfred self-assigned this May 18, 2023
@benfred
Copy link
Member

benfred commented May 18, 2023

@enp1s0 thanks for the heads up - I'm taking a look

benfred added a commit to benfred/raft that referenced this issue May 18, 2023
@benfred
Fix failing TiledKNNTest unittest
06e2bc8 
The TiledKNNTest test was faiiling - and it seems to be because
the matrix::select_k code isn't guaranteed to return elements in
sorted order. The test was expecting outputs to be sorted, and was
failing because of it. This change fixes the test to sort the
outputs before comparing.

Closes rapidsai#1526
@benfred benfred mentioned this issue May 18, 2023
Merged
@benfred
Copy link
Member

benfred commented May 18, 2023

It looks like the problem was just in the unittest - with the test expecting sorted outputs, and the matrix::select_k not guaranteeing that outputs will be sorted. Fix is in #1533

rapids-bot bot pushed a commit that referenced this issue May 18, 2023
@benfred
Fix failing TiledKNNTest unittest (#1533)
29d1c15 
The TiledKNNTest test was faiiling - and it seems to be because the matrix::select_k code isn't guaranteed to return elements in sorted order. The test was expecting outputs to be sorted, and was failing because of it. This change fixes the test to sort the outputs before comparing.

Closes #1526

Authors:
  - Ben Frederickson (https://github.com/benfred)
  - Corey J. Nolet (https://github.com/cjnolet)

Approvers:
  - Corey J. Nolet (https://github.com/cjnolet)

URL: #1533
@enp1s0 enp1s0 mentioned this issue May 19, 2023
Merged
@enp1s0
Copy link
Member Author

enp1s0 commented May 19, 2023

Thanks for fixing the bug!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@benfred benfred

Labels
bug Something isn't working
Projects
VS/ML/DM Primitives Release Board
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix failing TiledKNNTest unittest benfred/raft
2 participants
@benfred @enp1s0