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

add libkvikio wheel build to manifest #283

Merged
merged 9 commits into from
Jun 24, 2024
Merged

Conversation

msarahan
Copy link
Contributor

rapidsai/kvikio#369 moves the python wheel code one level deeper so that we add support for a dedicated libkvikio wheel. This adjustment needs to be accounted for here. Confused PR run without this change: https://github.com/rapidsai/kvikio/actions/runs/8900188332/job/24441209009#step:7:1446

rapidsai/kvikio#369 moves the python wheel code one level deeper so that we add support for a dedicated libkvikio wheel. This adjustment needs to be accounted for here. Confused PR run without this change: https://github.com/rapidsai/kvikio/actions/runs/8900188332/job/24441209009#step:7:1446
Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

We also need to bump the feature version here. While we're at it, I would also recommend adding the libkvikio directory since that's happening in the same kvikio PR.

@msarahan
Copy link
Contributor Author

msarahan commented May 7, 2024

I don't think this PR can pass without rapidsai/kvikio#369. @vyasr you wanted to merge this end of the loop first, right? Is it OK to merge with it failing here?

If we want things to be passing prior to merging, then I think we have to approach it on the kvikio side with the part that we ripped out here: https://github.com/rapidsai/kvikio/pull/369/files#diff-1eb4e5fd5611777d4e597ef299a1cb5ba8050c28a2dabbd4fbc56205d69e5ddaR70-R72

then merge this devcontainers PR, then remove the stuff from kvikio.

@vyasr
Copy link
Contributor

vyasr commented May 7, 2024

Yeah, one of these has to be merged while failing. Once the kvikio PR is passing all tests other than the devcontainer jobs, and once you have all the approvals that you need there, we can merge this PR and rerun tests a final time on that repo to make sure everything looks right before merging.

@trxcllnt trxcllnt changed the base branch from branch-24.06 to branch-24.08 June 10, 2024 22:01
@msarahan msarahan changed the title Account for kvikio python folder nesting add libkvikio wheel build to manifest Jun 17, 2024
@msarahan
Copy link
Contributor Author

@trxcllnt updated the kvikio path from rapidsai/kvikio#392 in #321

This PR now only adds an entry for building the cpp-only shared library wheel, which is utilized in rapidsai/kvikio#369

@msarahan
Copy link
Contributor Author

The kvikio PR has been merged. I'm going to close/reopen this PR to cycle CI and see where we stand.

@msarahan msarahan closed this Jun 19, 2024
@msarahan msarahan reopened this Jun 19, 2024
@vyasr vyasr merged commit 66c25e3 into rapidsai:branch-24.08 Jun 24, 2024
48 of 52 checks passed
rapids-bot bot pushed a commit to rapidsai/cuml that referenced this pull request Jul 16, 2024
This is a step towards adding support for dynamic linking with wheels (splitting the shared libraries out into their own wheels). That's being tracked in rapidsai/build-planning#33

This PR performs a necessary step of moving the cuml folder one level deeper, such that the python folder becomes a parent of multiple full-fledged projects, instead of having the python folder be the top level of one python project. This is split into this PR because this change affects so many files. It's easier to review the actual changes of supporting the split wheel when you don't have to also consider these moves.

This change also affects devcontainers, and there will need to be a change similar to rapidsai/devcontainers#283 for cuml.

Authors:
  - Mike Sarahan (https://github.com/msarahan)
  - Kyle Edwards (https://github.com/KyleFromNVIDIA)

Approvers:
  - James Lamb (https://github.com/jameslamb)
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: #5944
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants