Repository navigation
Optimize sum_rows_by_key() kernel #3176
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
Open
huuanhhuyn
wants to merge
17
commits into
NVIDIA:main
Choose a base branch
from
huuanhhuyn:improve_kmeans_fit
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+445
−46
Open
Changes from 2 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
666a617
Optimize sum_rows_by_key kernel
huuanhhuyn 7658a18
Keep the old kernel for K <=32. It is more optimized for that range.
huuanhhuyn 82fa848
Extra kernel - Sort rows by key before summing
huuanhhuyn eb08f52
Merge remote-tracking branch 'origin/main' into improve_kmeans_fit
huuanhhuyn a9ae4fa
Merge branch 'main' into improve_kmeans_fit
divyegala cad9556
Merge branch 'main' into improve_kmeans_fit
huuanhhuyn d6e7877
Add vectorized load kernel - improve latency-bound at K < 64, given 1…
huuanhhuyn f1f3b67
Single kernel selection point. Tune params.
huuanhhuyn 31e3608
Fix code rabbit
huuanhhuyn cf53129
Static assert weight and sum types
huuanhhuyn cc4af47
Reuse / extend libraries.
huuanhhuyn 30da49c
Dry-run compliance. No dry-run tests
huuanhhuyn 68538fe
Clang-format
huuanhhuyn 18d4d8a
Merge branch 'main' into improve_kmeans_fit
huuanhhuyn 1113f2b
Dry-run unit test for allocating path (sorted variant)
huuanhhuyn 2ef39df
Merge branch 'main' into improve_kmeans_fit
huuanhhuyn d858619
Batching
huuanhhuyn File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a static check that
SumsTdoes not narrowWeightTand that both types belong to the same numeric type family?This is to make sure casts like this don't cause accidental side-effects https://github.com/NVIDIA/raft/pull/3176/changes#diff-ee891f4e924cc29da43803011dfcae91ee2bcec531ba7a447cd0fa2ea5da6e01R376
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also don't see any circumstance where we want give weight type more precise or even of a different family type than the sum. I can add it.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added it cf53129.
I assume we still want to allow integer weight type with float sum type. Besides it, integers of different sizes should be fine as they don't cause precision issue.
-> So we throws when both are floats and
SumsTdoes not narrowWeightT.