[FEA] Remove "All columns required to have same data type" requirement in .to_dlpack() method.

[miguelusque]

Is your feature request related to a problem? Please describe.
Hi!

While trying to convert a cudf dataframe to a cupy array, I have noticed a different behaviour between two cudf methods:

• _df_.as_gpu_matrix()
• _df_.to_dlpack()

While the first method works well when the dataframe contains different numeric types (int64 and float64), the second method returns the following error:

RuntimeError: cuDF failure at: /opt/conda/envs/rapids/conda-bld/libcudf_1607613794356/work/cpp/src/interop/dlpack.cpp:199: All columns required to have same data type

The following code works:

import cudf
import cupy

df = cudf.DataFrame({'a': 3, 'b': 3.0})
arr = cupy.asarray(df.as_gpu_matrix())

The following code fails:

import cudf
import cupy

df = cudf.DataFrame({'a': 3, 'b': 3.0})
arr = cupy.fromDlpack(df.to_dlpack())

Hope it helps!
Miguel

Describe the solution you'd like
It would be great if _df_.to_dlpack() method could behave similarly to _df_.as_gpu_matrix() method.

Describe alternatives you've considered
Using _df_.as_gpu_matrix() method.

Additional context
Using RAPIDS 0.17, the latest stable version on Ubuntu 20.04.

[harrism]

This would require changing the C++ implementation, so I'd like to understand... what is the type of the data that as_gpu_matrix() returns in your example? Does it promote everything to float64? Does the user have control over the resulting data type?

This requires casts, so at the C++ level we would likely require the user to explicitly specify the output type, and reserve the right to error. We would still error if they try to do something impossible like convert a table with one column of ints and one column of strings (or lists of lists of strings!) into a tensor of floats.

[miguelusque]

Hi @harrism ,

In the example above, as_gpu_matrix() method casts everything to float64.

I understand casting is needed, but maybe it is well-worth to double-check how as_gpu_matrix() is implemented because I didn´t need to specify any output type when invoking it.

An additional output type parameter would be nice. Nevertheless, I would make it optional. When not present, I would cast to the bigger datatype which is able to store the data in the dataframe.

About the strings, I´d say there is no problem because, IIRC, dataframes with only numeric series are allowed so far.

[jrhemstad]

This would require changing the C++ implementation, so I'd like to understand... what is the type of the data that as_gpu_matrix() returns in your example? Does it promote everything to float64? Does the user have control over the resulting data type?

This requires casts, so at the C++ level we would likely require the user to explicitly specify the output type, and reserve the right to error. We would still error if they try to do something impossible like convert a table with one column of ints and one column of strings (or lists of lists of strings!) into a tensor of floats.

We should maintain the current C++ behavior of requiring everything to be the same type. It's the callers responsibility (i.e., the cuDF Python wrappers) to cast all the columns to the desired type beforehand.

[miguelusque]

Hi @jrhemstad ,

Can I ask why we should maintain current implementation? I mean, is there any technical reason why it couldn’t be changed?

[kkraus14]

@miguelusque I think the point thus far has been that the C++ function in libcudf shouldn't do any automatic casting as that's unexpected behavior from the perspective of C++.

On the Python side it could be argued that we should handle automatic casting for dlpack similar to what's done for as_gpu_matrix / .values.

[miguelusque]

HI @kkraus14 ,

Sure! My only concern is the performance penalty by performing the casting in the Python side instead of the c++ side (I do not have any data to support my concern).

I mean, in the worst scenario, the user will need to cast his data, so the fastest it could be done, the better.

In the Python side, it can always be invoked df.astype(cp.float64).to_dlpack() as a workaround, but I do not know the performance implications.

Nevertheless, thank you for considering this feature request!

[harrism]

The actual casting would be done on the C++ side, just explicitly invoked from the Python implementation on an as-needed basis.

[github-actions]

This issue has been marked stale due to no recent activity in the past 30d. 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 marked rotten if there is no activity in the next 60d.

[miguelusque]

I think this feature request is still relevant.

[github-actions]

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.

[miguelusque]

I think this issue is still relevant.

[github-actions]

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.

[miguelusque]

I think this feature request is still relevant.