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

Pin cudf dependencies during build #1901

Merged
merged 4 commits into from
Apr 2, 2024
Merged

Conversation

jlowe
Copy link
Member

@jlowe jlowe commented Mar 26, 2024

Closes #1666. This restores the changes from #1877 and updates the rapids-cmake version to avoid the issue reported in NVIDIA/spark-rapids#10627.

GaryShen2008 and others added 2 commits March 26, 2024 10:06
* Use rapids-cmake to pin cudf dependencies to known working SHA1's

* Use git download rapids-cmake

So that we can get SHA
Use same format of CopyRight
Use cudf/rapids_config.cmake which is renamed from fetch_rapids.cmake
Update versions to latest

Signed-off-by: Gary Shen <[email protected]>

* Commit cudf to the latest code before mvn verify

Signed-off-by: Gary Shen <[email protected]>

* Remove the first commit of cudf

since the second one can commit it at once

Signed-off-by: Gary Shen <[email protected]>

---------

Signed-off-by: Gary Shen <[email protected]>
Co-authored-by: Robert Maynard <[email protected]>
Signed-off-by: Jason Lowe <[email protected]>
@jlowe jlowe added the build label Mar 26, 2024
@jlowe jlowe self-assigned this Mar 26, 2024
@jlowe
Copy link
Member Author

jlowe commented Mar 26, 2024

build

@jlowe
Copy link
Member Author

jlowe commented Mar 26, 2024

Verified RAPIDS Accelerator zstd tests pass with this build.

gerashegalov
gerashegalov previously approved these changes Apr 2, 2024
Copy link
Collaborator

@gerashegalov gerashegalov left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@ttnghia ttnghia left a comment

Choose a reason for hiding this comment

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

Does this mean we always pin, and we have to manually upgrade after a while?

@jlowe
Copy link
Member Author

jlowe commented Apr 2, 2024

Does this mean we always pin, and we have to manually upgrade after a while?

No. There's two modes to the build. The default is to use the pinned versions, but the submodule sync build, which automatically runs every few hours, will build in a mode that updates the pinned versions file to whatever versions it finds when it moves to the updated cudf version. See https://github.com/NVIDIA/spark-rapids-jni/pull/1901/files#diff-a4b52f1881e34609ce5cd5e0722da2f2f708d1eef03334723a5628082fe94d31

@jlowe
Copy link
Member Author

jlowe commented Apr 2, 2024

I just realized I should update this PR to refresh the pin versions, since it's been almost a week since those were updated in this PR. Should have that updated shortly.

Signed-off-by: Jason Lowe <[email protected]>
@jlowe
Copy link
Member Author

jlowe commented Apr 2, 2024

build

@jlowe jlowe merged commit fa7c202 into NVIDIA:branch-24.04 Apr 2, 2024
3 checks passed
@jlowe jlowe deleted the cudfpin branch April 2, 2024 18:42
@bdice
Copy link
Contributor

bdice commented Apr 2, 2024

@jlowe This is awesome! I'm really glad this approach worked out.

@jlowe
Copy link
Member Author

jlowe commented Apr 2, 2024

Thank @bdice, but all the credit goes to @robertmaynard for the idea and implementation.

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

Successfully merging this pull request may close these issues.

[BUG] The build can be broken due to dependency changes of cudf
5 participants