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

Upgrade to latest cutlass version #2503

Merged
merged 10 commits into from
Nov 21, 2024

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Nov 20, 2024

This upgrade is important because cutlass 3.4 fixed issues with kernel visibility that would otherwise lead to global kernel symbols from raft kernels using cutlass.

@vyasr vyasr added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change and removed cpp CMake labels Nov 20, 2024
Copy link
Contributor

@robertmaynard robertmaynard left a comment

Choose a reason for hiding this comment

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

We should open a PR targetting 25.02 that refactors the logic in get_cutlass to follow that used in places like: https://github.com/rapidsai/cuvs/blob/branch-24.12/cpp/cmake/thirdparty/get_hnswlib.cmake

This allows for us to not have to manually manage the patch command ( and allows us to error when the patch required fails ).

@vyasr vyasr marked this pull request as ready for review November 21, 2024 03:14
@vyasr vyasr requested review from a team as code owners November 21, 2024 03:14
@vyasr vyasr requested a review from jameslamb November 21, 2024 03:14
@vyasr
Copy link
Contributor Author

vyasr commented Nov 21, 2024

I can't approve my own PR (even though Kyle did all the work) so @KyleFromNVIDIA can approve on my behalf if needed 😄 The changes all look good.

Copy link
Contributor

@KyleFromNVIDIA KyleFromNVIDIA left a comment

Choose a reason for hiding this comment

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

Approving my own work on @vyasr's behalf

Copy link
Contributor

@robertmaynard robertmaynard left a comment

Choose a reason for hiding this comment

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

The symbol_exclusions list needs to be dropped from the rapidsai/shared-workflows/.github/workflows/[email protected] CI jobs. That will also verify this will fix the cugraph issue

@KyleFromNVIDIA
Copy link
Contributor

/merge

@rapids-bot rapids-bot bot merged commit fbdf2df into rapidsai:branch-24.12 Nov 21, 2024
70 checks passed
rapids-bot bot pushed a commit to rapidsai/cuml that referenced this pull request Nov 22, 2024
Depends on rapidsai/raft#2503, which includes the kernel visibility fixes needed from cutlass.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Kyle Edwards (https://github.com/KyleFromNVIDIA)

URL: #6140
@vyasr vyasr deleted the feat/upgrade_cutlass branch December 5, 2024 00:02
rapids-bot bot pushed a commit to rapidsai/cuvs that referenced this pull request Dec 5, 2024
Due to the tight integration between cuvs and raft, we need to ensure that cuvs is updated for rapidsai/raft#2503 or builds of cuvs that rely on cloning raft will get an incompatible version of cutlass due to raft's update.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Kyle Edwards (https://github.com/KyleFromNVIDIA)
  - Ben Frederickson (https://github.com/benfred)

URL: #516
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CMake cpp improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants