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

JNI Bindings to fetch CUDA compute capability versions. #10568

Merged
merged 2 commits into from
Apr 1, 2022
Merged

JNI Bindings to fetch CUDA compute capability versions. #10568

merged 2 commits into from
Apr 1, 2022

Conversation

mythrocks
Copy link
Contributor

@mythrocks mythrocks commented Apr 1, 2022 •
edited
Loading

This commit introduces JNI bindings to retrieve the major and minor CUDA compute capability versions for the current CUDA device.

This feature enables introspection from spark-rapids to detect the GPU architecture, for model-specific behaviour.
This is required from NVIDIA/spark-rapids/pull/5122, to work around the erroneous behaviour of JNI fixed_width_convert_to_rows() on Pascal GPUs (#10569), (which in turn produces failures like NVIDIA/spark-rapids/issues/4980).

@mythrocks mythrocks requested a review from a team as a code owner April 1, 2022 04:34
@mythrocks mythrocks self-assigned this Apr 1, 2022
@github-actions github-actions bot added the Java Affects Java cuDF API. label Apr 1, 2022
@mythrocks mythrocks added 3 - Ready for Review Ready for review by team 4 - Needs cuDF (Java) Reviewer and removed Java Affects Java cuDF API. labels Apr 1, 2022
@mythrocks mythrocks added feature request New feature or request improvement Improvement / enhancement to an existing function non-breaking Non-breaking change and removed improvement Improvement / enhancement to an existing function labels Apr 1, 2022
@github-actions github-actions bot added the Java Affects Java cuDF API. label Apr 1, 2022
@mythrocks
JNI Bindings to fetch CUDA compute capability versions.
dfd9975 
Signed-off-by: MithunR <mythrocks@gmail.com>
@mythrocks mythrocks mentioned this pull request Apr 1, 2022
Merged
@codecov
Copy link

codecov bot commented Apr 1, 2022 •
edited
Loading

Codecov Report

Merging #10568 (c5fe1a2) into branch-22.04 (a02b7c2) will increase coverage by 0.01%.
The diff coverage is n/a.

@@               Coverage Diff                @@
##           branch-22.04   #10568      +/-   ##
================================================
+ Coverage         86.15%   86.17%   +0.01%     
================================================
  Files               141      141              
  Lines             22510    22510              
================================================
+ Hits              19394    19397       +3     
+ Misses             3116     3113       -3
Impacted Files Coverage Δ
python/cudf/cudf/core/column/numerical.py 95.33% <0.00%> (-0.30%) ⬇️
python/cudf/cudf/core/column/string.py 88.91% <0.00%> (+0.12%) ⬆️
python/cudf/cudf/core/groupby/groupby.py 91.72% <0.00%> (+0.22%) ⬆️
python/cudf/cudf/core/tools/datetimes.py 84.49% <0.00%> (+0.30%) ⬆️
python/cudf/cudf/core/column/lists.py 90.69% <0.00%> (+0.46%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a02b7c2...c5fe1a2. Read the comment docs.

jlowe
jlowe reviewed Apr 1, 2022
View reviewed changes
java/src/main/native/src/CudaJni.cpp Outdated Show resolved Hide resolved
java/src/main/java/ai/rapids/cudf/Cuda.java Outdated Show resolved Hide resolved
@mythrocks
Review comments:
c5fe1a2 
1. Moved implementation inline. Removed util function.
2. Renamed to getComputeCapabilityM{aj,in}or.
@mythrocks
Copy link
Contributor Author

I'm running the (spark-rapids) tests again against Pascal and non-Pascal, with this change in place. I'll confirm here if/when things look agreeable.

@ttnghia
Copy link
Contributor

ttnghia commented Apr 1, 2022

Is this coming to the (frozen) 22.04? Or it was a mistake?

@mythrocks
Copy link
Contributor Author

Is this coming to the (frozen) 22.04? Or it was a mistake?

We're working on addressing NVIDIA/spark-rapids/issues/4980 in the 22.04 timeframe.

@mythrocks
Copy link
Contributor Author

mythrocks commented Apr 1, 2022 •
edited
Loading

I'm running the (spark-rapids) tests again against Pascal and non-Pascal, with this change in place. I'll confirm here if/when things look agreeable.

Tested on Pascal and Turing. Verified that this satisfies the intent of NVIDIA/spark-rapids/pull/5122.

@mythrocks
Copy link
Contributor Author

mythrocks commented Apr 1, 2022 •
edited
Loading

Updated the description for more context.

I'm not sure I agree that the whole story needs to be in the commit message (rather than, say, in a comment), but I'll put it there for convenience.

@ajschmidt8 ajschmidt8 merged commit 2c81bed into rapidsai:branch-22.04 Apr 1, 2022
@vyasr vyasr added 4 - Needs Review Waiting for reviewer to review or respond and removed 4 - Needs cuDF (Java) Reviewer labels Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlowe jlowe jlowe approved these changes

@ttnghia ttnghia ttnghia approved these changes

@nvdbaranec nvdbaranec nvdbaranec approved these changes

Assignees

@mythrocks mythrocks

Labels
3 - Ready for Review Ready for review by team 4 - Needs Review Waiting for reviewer to review or respond feature request New feature or request Java Affects Java cuDF API. non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mythrocks @ttnghia @jlowe @nvdbaranec @vyasr @ajschmidt8