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

[BUG] Builds are not reproducible #11533

Closed
revans2 opened this issue Aug 15, 2022 · 11 comments
Closed

[BUG] Builds are not reproducible #11533

revans2 opened this issue Aug 15, 2022 · 11 comments
Assignees
Labels
2 - In Progress Currently a work in progress bug Something isn't working Spark Functionality that helps Spark RAPIDS

Comments

@revans2
Copy link
Contributor

revans2 commented Aug 15, 2022

Describe the bug
I tried to redo an older 22.10 build from a specific SHA hash and it failed. see NVIDIA/spark-rapids-jni#479 It looks like cudf always pulls the latest version of rapids-cmake. So if rapids-cmake ever changes older builds are impacted and we have no way to keep the exact version of rapids-cmake.

Expected behavior
I check out a specific version of cudf from git it always builds exactly the same. Code does not magically change on me without my knowledge.

If CUDF always wants to run on the latest version, that is fine. But then we need a way to be able to set the exact SHA version we want for rapids-cmake.

@revans2 revans2 added bug Something isn't working Needs Triage Need team to review and classify Spark Functionality that helps Spark RAPIDS labels Aug 15, 2022
@robertmaynard
Copy link
Contributor

It looks like cudf always pulls the latest version of rapids-cmake.
cudf on a given branch ( 22.10 ) will pull the latest version of rapids-cmake on the same branch. Likewise for dependencies like rmm which are also pulled via branch names.

If you need to test with a specific version of rapids-cmake you can follow this guide https:/rapidsai/rapids-cmake#overriding-rapidscmake which shows how specify a specific version ( GIT_TAG can also be a SHA ). You would place this in the fetch_rapids.cmake before any other statement.

@revans2
Copy link
Contributor Author

revans2 commented Aug 29, 2022

@robertmaynard I see that this was closed and then re-opened again. Are there plans to do more for this? The docs you pointed us to I think are workable, and we can probably patch CUDF as we build it to get a consistent version. But if you are going to do more, like expose the version through a CMAKE variable or something it would be good to understand that, because I don't want to patch CUDF unless I have to.

@robertmaynard
Copy link
Contributor

I accidently closed, and didn't want to come across as presumptious so I re-opened.

The balance between ease of use and reproducibility for 'nightly' builds is hard, and we currently go towards ease of use. We could move over to explicit SHA's but we would need to develop some infrastructure first. Primarily we need a robust way to ensure that during code-freeze all projects have synced to the same rapids-cmake SHA.

@revans2
Copy link
Contributor Author

revans2 commented Aug 30, 2022

I was thinking of something very simple. Like an environment variable for the SHA and if it is not set, then it defaults to what happens today. I realize that for most of rapids and CUDF in particular that ease of use is paramount. All I would like is a way to be able to set this without having to patch CUDF itself. I could even write up the patch myself. I mostly want to have someones blessing that this is acceptable.

@robertmaynard
Copy link
Contributor

The ability to pin rapids-cmake is coming to all consumers via rapidsai/rapids-cmake#257 which will allow you to pin by passing -Drapids-cmake-sha=<sha>

@github-actions
Copy link

github-actions bot commented Oct 6, 2022

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

@robertmaynard
Copy link
Contributor

Closing since rapidsai/rapids-cmake#257 has been merged

@bdice
Copy link
Contributor

bdice commented Jan 19, 2024

@robertmaynard We reopened this issue following discussion with the Spark team. Is there work to be done for cudf specifically, or would all the work be in rapids-cmake and spark-rapids-jni?

@robertmaynard
Copy link
Contributor

@robertmaynard We reopened this issue following discussion with the Spark team. Is there work to be done for cudf specifically, or would all the work be in rapids-cmake and spark-rapids-jni?

No work in libcudf, all the work will be in rapids-cmake and spark-rapids-jni

@vyasr
Copy link
Contributor

vyasr commented May 14, 2024

I'm closing this since I believe all the requirements were addressed in rapidsai/rapids-cmake#530 and its follow-ups. @robertmaynard please reopen if I am mistaken.

@vyasr vyasr closed this as completed May 14, 2024
@robertmaynard
Copy link
Contributor

I'm closing this since I believe all the requirements were addressed in rapidsai/rapids-cmake#530 and its follow-ups. @robertmaynard please reopen if I am mistaken.

You are correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 - In Progress Currently a work in progress bug Something isn't working Spark Functionality that helps Spark RAPIDS
Projects
None yet
Development

No branches or pull requests

5 participants