Skip to content

Use 1D grid calculations in epsilon_neighborhood#1847

Merged
rapids-bot[bot] merged 7 commits intorapidsai:mainfrom
divyegala:epsilon_neighborhood_1d
Mar 4, 2026
Merged

Use 1D grid calculations in epsilon_neighborhood#1847
rapids-bot[bot] merged 7 commits intorapidsai:mainfrom
divyegala:epsilon_neighborhood_1d

Conversation

@divyegala
Copy link
Member

@divyegala divyegala commented Feb 25, 2026

Closes rapidsai/cuml#5393 closes rapidsai/cuml#7676

This was necessary to bypass the limits of grid.y < 65536, as cuml passes n_rows to the n_cols dimension in the usage of this API.

@divyegala divyegala self-assigned this Feb 25, 2026
@divyegala divyegala requested a review from a team as a code owner February 25, 2026 02:05
@divyegala divyegala added bug Something isn't working non-breaking Introduces a non-breaking change labels Feb 25, 2026
Copy link
Member

@dantegd dantegd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, though before approving explicitly wanted to ask if you think it'd be worth adding a explicit test to exercise this?

@divyegala
Copy link
Member Author

Looks good to me, though before approving explicitly wanted to ask if you think it'd be worth adding a explicit test to exercise this?

@dantegd good idea. Will add 👍

Copy link
Contributor

@tarang-jain tarang-jain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what is being done here. LGTM except possibly one minor optimization

Copy link
Contributor

@viclafargue viclafargue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Divye! LGTM. An alternative solution would have been to swap the x and y dimensions. The number of features divided by Policy::Mblk should rarely ever go beyond the limit of 65535. This would avoid adding an argument to the kernel and also avoid the two small instructions to get block_x and block_y back. But, I agree that the current solution may be more robust.

@divyegala
Copy link
Member Author

@viclafargue That solution does not work because we cannot guarantee always that m will be passed as n. cuML itself reverses the values of m and n before calling this function, but cuVS does not expect the reversal. So if anyone else uses the API then you run into the same problem.

@divyegala
Copy link
Member Author

/merge

@rapids-bot rapids-bot bot merged commit 105c61e into rapidsai:main Mar 4, 2026
125 of 130 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working non-breaking Introduces a non-breaking change

4 participants