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] Usage of copy constructor in column.hpp breaks compilation #8530

Closed
wmalpica opened this issue Jun 16, 2021 · 3 comments
Closed

[BUG] Usage of copy constructor in column.hpp breaks compilation #8530

wmalpica opened this issue Jun 16, 2021 · 3 comments
Labels
bug Something isn't working

Comments

@wmalpica
Copy link

Describe the bug
Building blazingsql requires to include cudf/table/table.hpp which in turn includes cudf/column/column.hpp

After the changes in rmm https://github.com/rapidsai/rmm/pull/775/files
The copy constructor now requires providing a cuda_stream_view, which is not being provided in line 104:

include/cudf/column/column.hpp:104:36: error: use of deleted function ‘rmm::device_buffer::device_buffer(const rmm::device_buffer&)’
       _children{std::move(children)}

Steps/Code to reproduce bug
I am trying to update the blazingsql code to account for the changes in
https://github.com/rapidsai/rmm/pull/775/files
If you try to build my branch, you will see the same error:

git clone https://github.com/wmalpica/blazingsql.git
cd blazingsql
git checkout fix/update_rmm_device_buffer
./dependencies.sh 21.08 11.2 nightly

Expected behavior
It should build

Environment overview (please complete the following information)
Baremetal. cudf installed using conda. Versions in use:
cmake 3.18.5 h1f3970d_0 rapidsai-nightly
cudf 21.08.00a210616 cuda_11.2_py38_g91c727f396_166 rapidsai-nightly
dask-cuda 21.08.00a210615 py38_27 rapidsai-nightly
dask-cudf 21.08.00a210616 py38_g91c727f396_166 rapidsai-nightly
libcudf 21.08.00a210616 cuda11.2_g15b18f46e8_167 rapidsai-nightly
librmm 21.08.00a210615 cuda11.2_g91491aa_25 rapidsai-nightly
rmm 21.08.00a210615 cuda_11.2_py38_g91491aa_25 rapidsai-nightly
ucx 1.9.0+gcd9efd3 cuda11.2_0 rapidsai-nightly
ucx-proc 1.0.0 gpu rapidsai-nightly
ucx-py 0.20.0a210615 py38_gcd9efd3_41 rapidsai-nightly

Environment details
Please run and paste the output of the cudf/print_env.sh script here, to gather any other relevant environment details

Additional context
Add any other context about the problem here.

@wmalpica wmalpica added bug Something isn't working Needs Triage Need team to review and classify labels Jun 16, 2021
@harrism
Copy link
Member

harrism commented Jun 16, 2021

Hi @wmalpica, can you paste the line of code that calls the column ctor (line 104)? I suspect that you just need to wrap the argument to the null_mask parameter of that ctor with std::move. (note the std::forward in the implementation -- if you don't move it, the forward will result in a copy, which I suspect is what is calling the deleted device_buffer copy ctor.

I had to change a number of these in libcudf after changing rmm::device_buffer. For example:

https://github.com/rapidsai/cudf/pull/8280/files#diff-1140f9642e1a1dcc42c6830acb3f3681eb71f63f36cd7444cfb692e6c487777bL55-L57

Basically, rapidsai/rmm#775 uncovered a number of places where we were copying but should have been moving.

@wmalpica
Copy link
Author

Thanks @harrism , you are correct, I was simply not moving the data. I was so focused on the changes to the cuda_stream_view not being set by default, I did not really see what was happening. Sorry to waste your time. This is great, now we too have noticed when we were inadvertently copying data.

@harrism
Copy link
Member

harrism commented Jun 16, 2021

No worries, happy to help!

@bdice bdice removed the Needs Triage Need team to review and classify label Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants