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.

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

Refactor spectral scale_obs to use existing normalization function #2319

Conversation

ChuckHastings
Copy link
Contributor

The scale_obs function was calling a custom kernel to scale the elements of a matrix column by the l2-norm of the column.

There were two issues:

  1. The kernel launch parameters would go out of bounds if the graph was too large. The Y dimension is limited to 65535, but there was no logic in the function to ensure that we didn't set the Y value larger than that
  2. A bug in the kernel, the column norm was not being calculated correctly... the outer loop was terminating, hence we were really only computing the column norm of the last column in the block. Then we were normalizing all columns in the block by that value instead of by each value.

To simplify (there's going to be some optimization work done this summer), I replaced this with a simple thrust call that will scale the values correctly.

@ChuckHastings ChuckHastings requested a review from a team as a code owner May 15, 2024 20:16
@github-actions github-actions bot added the cpp label May 15, 2024
@cjnolet cjnolet added bug Something isn't working non-breaking Non-breaking change labels May 15, 2024
@ChuckHastings ChuckHastings changed the title Refactor spectral scale_obs to use thrust instead of custom kernel Refactor spectral scale_obs to use existing normalization function May 16, 2024
@cjnolet
Copy link
Member

cjnolet commented May 17, 2024

/merge

@rapids-bot rapids-bot bot merged commit 12f0096 into rapidsai:branch-24.06 May 17, 2024
72 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 cpp non-breaking Non-breaking change
Projects
Development

Successfully merging this pull request may close these issues.

2 participants