rapidsai / cudf

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

[FEA] Clean up `type_to_name` and add `print_type` debug utility #17376

Open PointKernel opened 4 days ago

PointKernel commented 4 days ago

Is your feature request related to a problem? Please describe. PR #17194 introduces a type_id to string utility located within the benchmark code.

@ttnghia identified that this could be a shared utility for both benchmarks and tests, as it could also be used to create a print_type utility for debugging purposes (see: https://github.com/rapidsai/cudf/pull/17194/files#r1849064965). @davidwendt highlighted that this feature is redundant and performs a similar function to type_to_name.

Describe the solution you'd like

davidwendt commented 4 days ago

Just wanted to point out the there is a type_to_name function already defined here https://github.com/rapidsai/cudf/blob/56061bdd7988608ff45ae34f88ae9ddd77c9e6b4/cpp/include/cudf/utilities/type_dispatcher.hpp#L148 This does the string-ify but does not have print utility.

ttnghia commented 3 days ago

Oh great, I didn't notice that for such a long time.

However, we have:

std::string type_to_name(data_type type) { return type_dispatcher(type, type_to_name_impl{}); }

in type_dispatcher.cpp. This is a misleading place indeed.

PointKernel commented 3 days ago

I've revised the issue description to incorporate the latest insights from our discussions. Thank you both.