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

Update nvcomp to 3.0.4 (includes API changes) #314

Merged
merged 27 commits into from
Nov 9, 2023
Merged

Update nvcomp to 3.0.4 (includes API changes) #314

merged 27 commits into from
Nov 9, 2023

Conversation

vuule
Copy link
Contributor

@vuule vuule commented Nov 3, 2023 •
edited
Loading

Update the nvCOMP version used for compression/decompression to 3.0.4.

See also:

rapidsai/cudf#13815
rapidsai/rapids-cmake#451

@vuule vuule self-assigned this Nov 3, 2023
@vuule vuule added breaking Introduces a breaking change feature request New feature or request labels Nov 3, 2023
@vuule vuule mentioned this pull request Nov 3, 2023
Closed
@vuule
Copy link
Contributor Author

vuule commented Nov 3, 2023

reverted set_scratch_allocators to at least see CI passing with nvcomp 3.0.4, even with missing API

@vuule
Copy link
Contributor Author

vuule commented Nov 3, 2023

omg, there's more
all managers have a new parameter in the constuctors

@madsbk madsbk mentioned this pull request Nov 8, 2023
Closed
@vuule vuule changed the title TEST nvcomp 3 changes Upgrade to nvCOMP 3.0.4 (includes API changes) Nov 9, 2023
@vuule vuule changed the title Upgrade to nvCOMP 3.0.4 (includes API changes) Update nvcomp to 3.0.4 (includes API changes) Nov 9, 2023
bdice
bdice approved these changes Nov 9, 2023
View reviewed changes
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Approving, with two notes about future changes and how we will sort them out.

cpp/cmake/fetch_rapids.cmake
Comment on lines +14 to +15
set(rapids-cmake-repo vuule/rapids-cmake)
set(rapids-cmake-branch upgrade-nvcomp-3.0.0)
Copy link
Contributor

Choose a reason for hiding this comment

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

After rapidsai/rapids-cmake#451 is merged, revert these lines (before merging this PR).

Suggested change
set(rapids-cmake-repo vuule/rapids-cmake)
set(rapids-cmake-branch upgrade-nvcomp-3.0.0)

Copy link
Contributor

Choose a reason for hiding this comment

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

I forgot to commit this before merging. It's in #316.

.github/workflows/pr.yaml
@@ -18,7 +18,7 @@ jobs:
- conda-python-build
- conda-python-tests
- docs-build
- devcontainer
# - devcontainer
Copy link
Contributor

Choose a reason for hiding this comment

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

Devcontainer CI will have to remain disabled until cudf is updated with rapidsai/cudf#13815, due to the conflicting nvcomp dependency which creates circular conflicts between cudf and kvikio.

madsbk
madsbk approved these changes Nov 9, 2023
View reviewed changes
Copy link
Member

@madsbk madsbk left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @vuule

@bdice bdice marked this pull request as ready for review November 9, 2023 16:42
@bdice bdice requested review from a team as code owners November 9, 2023 16:42
bdice
bdice approved these changes Nov 9, 2023
View reviewed changes
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Re-approving since this was in draft before.

@bdice
Copy link
Contributor

bdice commented Nov 9, 2023 •
edited
Loading

@madsbk Can you re-approve on behalf of Python codeowners? This was in draft so your prior approval won't count, I think.

edit: Nevermind, it seems to have accepted your prior approval.

@bdice
Copy link
Contributor

bdice commented Nov 9, 2023

/merge

@rapids-bot rapids-bot bot merged commit b8f6218 into rapidsai:branch-23.12 Nov 9, 2023
@bdice bdice mentioned this pull request Nov 9, 2023
Merged
raydouglass pushed a commit that referenced this pull request Nov 9, 2023
@bdice
Revert rapids-cmake branch. (#316)
e1762f2 
Accidentally didn't commit this change in #314.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bdice bdice bdice approved these changes

@madsbk madsbk madsbk approved these changes

@raydouglass raydouglass raydouglass approved these changes

Assignees

@vuule vuule

Labels
breaking Introduces a breaking change feature request New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vuule @bdice @madsbk @raydouglass