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

Eliminate literal parameters to uvector::set_element_async and device_scalar::set_value #7563

Merged
merged 3 commits into from
Mar 19, 2021

Conversation

harrism
Copy link
Member

@harrism harrism commented Mar 10, 2021

After rapidsai/rmm#725 is merged, this PR updates cuspatial to eliminate passing literal values to device_scalar::set_value.

Note there are still similar issues with uses of cudf::scalar that need to be addressed. But I wanted to get this open before the end of the week. This can be merged after RMM 725 is merged to fix the build, so I think the scalar issues should be fixed separately.

@harrism harrism added 3 - Ready for Review Ready for review by team improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Mar 10, 2021
@harrism harrism requested a review from a team as a code owner March 10, 2021 23:33
@harrism harrism self-assigned this Mar 10, 2021
@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Mar 10, 2021
Comment on lines +52 to +55
auto const in{4};
auto const out{1};
input->set_value(in);
output->set_value(out);
Copy link
Contributor

Choose a reason for hiding this comment

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

why this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

@harrism
Copy link
Member Author

harrism commented Mar 17, 2021

This will not pass CI until rapidsai/rmm#725 is merged.

@harrism harrism marked this pull request as draft March 18, 2021 23:08
@harrism harrism marked this pull request as ready for review March 18, 2021 23:08
@harrism harrism requested review from jrhemstad, nvdbaranec and devavret and removed request for jrhemstad March 18, 2021 23:09
@harrism
Copy link
Member Author

harrism commented Mar 18, 2021

rerun tests

@galipremsagar
Copy link
Contributor

rerun tests

@kkraus14 kkraus14 added the 5 - Ready to Merge Testing and reviews complete, ready to merge label Mar 19, 2021
@harrism
Copy link
Member Author

harrism commented Mar 19, 2021

@gpucibot merge

@codecov
Copy link

codecov bot commented Mar 19, 2021

Codecov Report

Merging #7563 (3a1bcde) into branch-0.19 (7871e7a) will increase coverage by 0.62%.
The diff coverage is 93.04%.

❗ Current head 3a1bcde differs from pull request most recent head 4ab14dd. Consider uploading reports for the commit 4ab14dd to get more accurate results
Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.19    #7563      +/-   ##
===============================================
+ Coverage        81.86%   82.49%   +0.62%     
===============================================
  Files              101      101              
  Lines            16884    17402     +518     
===============================================
+ Hits             13822    14355     +533     
+ Misses            3062     3047      -15     
Impacted Files Coverage Δ
python/cudf/cudf/core/index.py 93.45% <ø> (+0.59%) ⬆️
python/cudf/cudf/core/series.py 91.69% <ø> (+0.90%) ⬆️
python/cudf/cudf/core/tools/datetimes.py 84.53% <ø> (+0.08%) ⬆️
python/cudf/cudf/utils/cudautils.py 52.94% <ø> (+2.55%) ⬆️
python/cudf/cudf/utils/dtypes.py 89.88% <ø> (+0.37%) ⬆️
python/dask_cudf/dask_cudf/io/orc.py 91.04% <ø> (+0.13%) ⬆️
python/cudf/cudf/core/scalar.py 87.24% <85.71%> (+0.14%) ⬆️
python/cudf/cudf/core/column/numerical.py 94.83% <87.50%> (-0.20%) ⬇️
python/cudf/cudf/core/column/column.py 87.86% <90.00%> (+0.10%) ⬆️
python/cudf/cudf/core/column/decimal.py 92.75% <90.32%> (-2.12%) ⬇️
... and 59 more

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 561f68a...4ab14dd. Read the comment docs.

@galipremsagar
Copy link
Contributor

rerun tests

@rapids-bot rapids-bot bot merged commit 6fe8b1b into rapidsai:branch-0.19 Mar 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants