rapidsai / cudf

cuDF - GPU DataFrame Library
https://docs.rapids.ai/api/cudf/stable/
Apache License 2.0
8.45k stars 908 forks source link

Update operators to use the `type_dispatcher` #393

Closed eyalroz closed 5 years ago

eyalroz commented 6 years ago

Soon, PR #379 will be merged (one way or another), and we will be able to use templates instead of switch statements to implement operators taking type-erased gdf_column's.

I suggest we use this issue to 'hang' commits which make this transition for different operators, and close it when they're all done.

I'll begin with what the locations where I see such a transition being appropriate:

https://github.com/rapidsai/cudf/blob/master/cpp/src/io/csv/csv_reader.cu#L164

https://github.com/rapidsai/cudf/blob/master/cpp/src/io/csv/csv_reader.cu#L764

https://github.com/rapidsai/cudf/blob/master/cpp/src/io/csv/csv_reader.cu#L1056

https://github.com/rapidsai/cudf/blob/c9fbc8b275315d173e4345e10283b6c697fa7556/cpp/src/quantiles/quantiles.cu#L156

https://github.com/rapidsai/cudf/blob/c9fbc8b275315d173e4345e10283b6c697fa7556/cpp/src/sort/segmented_sorting.cu#L241

https://github.com/rapidsai/cudf/blob/c9fbc8b275315d173e4345e10283b6c697fa7556/cpp/src/join/joining.cu#L147

https://github.com/rapidsai/cudf/blob/c9fbc8b275315d173e4345e10283b6c697fa7556/cpp/src/unary/unary_ops.cu#L94 & below

https://github.com/rapidsai/cudf/blob/c9fbc8b275315d173e4345e10283b6c697fa7556/cpp/tests/utilities/cudf_test_utils.cuh#L78

https://github.com/rapidsai/cudf/blob/branch-0.5/cpp/tests/filter/helper/utils.cuh#L19

https://github.com/rapidsai/cudf/blob/b7356d9204acc5922a54fd7814c105390aaaafa5/cpp/src/filter/filter_ops.cu#L258

eyalroz commented 5 years ago

@harrism : I still see switches over the dtype in quantiles.cu, sorting.cu, segmented_sorting.cu (there's also binary_ops.cu but there it's understandable, perhaps). Consider reopening?

devavret commented 5 years ago

@eyalroz where? Are you sure you’re looking at branch 0.7? E.g. I don’t see it in quantiles.cu

eyalroz commented 5 years ago

@devavret :

quantiles.cu: gdf_quantile_approx() sorting.cu: gdf_radixsort_generic(). Note that I'm planning to add support for type-dispatching with implementations for only some of the gdf_dtype values; but you can still pull it off by defaulting to returning GDF_UNSUPPORTED_TYPE in the non-specialized templated code. segmented_sorting.cu: gdf_segmented_radixsort_generic()

devavret commented 5 years ago

@eyalroz can you please provide the GitHub link to those lines, for some reason I cannot find it.

eyalroz commented 5 years ago

@devavret : Oh, no, I got my branch origins mixed up. I take it back... this was correctly closed. Sorry for wasting your time.