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

Allow some of the sparse utility functions to handle larger matrices #2541

Open
wants to merge 22 commits into
base: branch-25.02
Choose a base branch
from
Open

Allow some of the sparse utility functions to handle larger matrices #2541

wants to merge 22 commits into from

Conversation

viclafargue
Copy link
Contributor

@viclafargue viclafargue requested a review from a team as a code owner January 14, 2025 15:22
@github-actions github-actions bot added the cpp label Jan 14, 2025
@copy-pr-bot copy-pr-bot
Copy link

copy-pr-bot bot commented Jan 16, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@pablete
Copy link

pablete commented Jan 22, 2025

Any updates here?
Thanks for working on the fix for very large matrices!

@viclafargue viclafargue added bug Something isn't working non-breaking Non-breaking change labels Jan 22, 2025
@viclafargue viclafargue mentioned this pull request Jan 22, 2025
Open
@viclafargue
Copy link
Contributor Author

viclafargue commented Jan 22, 2025 •
edited
Loading

Any updates here? Thanks for working on the fix for very large matrices!

These updates should fix some of the RAFT utilities to handle larger matrices and allow cuML's UMAP to process very large datasets. It is ready for review.

@viclafargue viclafargue changed the title Fix sparse utilities issues with large matrices Allow some of the sparse utility functions to handle larger matrices Jan 22, 2025
@dantegd dantegd added the improvement Improvement / enhancement to an existing function label Jan 22, 2025
@cjnolet cjnolet removed the improvement Improvement / enhancement to an existing function label Jan 23, 2025
@cjnolet
Copy link
Member

cjnolet commented Jan 23, 2025

/ok to test

@dantegd
Copy link
Member

dantegd commented Jan 28, 2025

/ok to test

@dantegd
Copy link
Member

dantegd commented Jan 28, 2025

/ok to test

dantegd
dantegd approved these changes Jan 28, 2025
View reviewed changes
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.

changes lgtm, @wphicks @divyegala maybe you want to take a second look, but tests seems to pass fine all around

@cjnolet
Copy link
Member

cjnolet commented Jan 28, 2025

Thanks @dantegd. I've asked @viclafargue to test the cuML side to make sure the hardcoded changes from uint32 to uint64 aren't going to cause any perf regressions or concerns.

cjnolet
cjnolet requested changes Jan 29, 2025
View reviewed changes
Copy link
Member

@cjnolet cjnolet left a comment

Choose a reason for hiding this comment

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

I've pointed out the places where we should be creating new templates. Please let me know if myself or someone else can help update these in the downstream libraries.

@divyegala
Copy link
Member

/ok to test

@divyegala
Copy link
Member

/ok to test

@cjnolet
Copy link
Member

cjnolet commented Jan 30, 2025

/ok to test

@divyegala
Copy link
Member

/ok to test

@divyegala divyegala mentioned this pull request Jan 30, 2025
Open
@divyegala
Copy link
Member

/ok to test

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

@cjnolet cjnolet cjnolet requested changes

@dantegd dantegd dantegd approved these changes

Requested changes must be addressed to merge this pull request.

Assignees

@viclafargue viclafargue

Labels
bug Something isn't working cpp non-breaking Non-breaking change
Projects
VS/ML/DM Primitives Release Board
Status: In Progress
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@viclafargue @pablete @cjnolet @dantegd @divyegala