cudf
cudf copied to clipboard
Add `gdb` pretty-printers for simple types
Description
This adds gdb
pretty printers for rmm::device_uvector
, thrust::*_vector
, thrust::device_reference
and cudf::*_span
. The implementation is based on https://github.com/NVIDIA/thrust/pull/1631. I will probably backport the thrust-specific changes to there as well, but since the location of the thrust source is not fixed, I'd prefer having all types in a self-contained file.
Checklist
- [x] I am familiar with the Contributing Guidelines.
- [x] New or existing tests cover these changes.
- [x] The documentation is up to date with these changes.
This seems to be working for all the cases I tested. The csv reader is a good place to try them out, since it uses host_span, device_span and device_uvector.
Is this the right home for all of this? You already added pretty printers for thrust in thrust. Pretty printers for RMM should probably live in RMM so more of RAPIDS can benefit?
I believe it would be hard to import dependencies from other components, since they may be placed in an arbitrary location (build/$config/_deps/.../scripts/gdb-pretty-printers.py in rapids-compose if pulling it via CPM, somewhere else if using an existing library). If I wanted to use pretty-printers from the individual files, I'd have to run
source path/to/rmm/scripts/gdb-pretty-printer.py
source path/to/thrust/scripts/gdb-pretty-printer.py
source path/to/cudf/scripts/gdb-pretty-printer.py
manually instead of just sourcing a single file.
All pretty-printers here rely on DeviceIterator
and HostIterator
, so they need to either pull in the file containing them or we duplicate the code across all of them.
OTOH, you mentioned thrust::*_vector
no longer being used inside cudf, so maybe this isn't too much of a roadblock after all?
Selfishly, I want to be able to find RMM pretty printers in RMM so that I can use them there and in other libraries. So I guess it's OK to duplicate them in cuDF, but I think they should be added to RMM as well.
Codecov Report
:exclamation: No coverage uploaded for pull request base (
branch-22.10@65a7821
). Click here to learn what that means. Patch has no changes to coverable lines.
Additional details and impacted files
@@ Coverage Diff @@
## branch-22.10 #11499 +/- ##
===============================================
Coverage ? 86.41%
===============================================
Files ? 145
Lines ? 22993
Branches ? 0
===============================================
Hits ? 19870
Misses ? 3123
Partials ? 0
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
I'm also in favor of putting these in their respective repos. Perhaps a script can be used to wrap the
I believe it would be hard to import dependencies from other components, since they may be placed in an arbitrary location (build/$config/_deps/.../scripts/gdb-pretty-printers.py in rapids-compose if pulling it via CPM, somewhere else if using an existing library). If I wanted to use pretty-printers from the individual files, I'd have to run
source path/to/rmm/scripts/gdb-pretty-printer.py source path/to/thrust/scripts/gdb-pretty-printer.py source path/to/cudf/scripts/gdb-pretty-printer.py
manually instead of just sourcing a single file.
Could we not just have a script to do these calls? I'd rather not have duplicates to maintain.
I totally see the issue - we could configure the rmm and thrust path via CMake and create the pretty-printer/load script inside build
. Would that be an acceptable solution?
I totally see the issue - we could configure the rmm and thrust path via CMake and create the pretty-printer/load script inside
build
. Would that be an acceptable solution?
I think that makes sense. These are only useful if you do a debug build after all.
Considering the horrible things I have to do to try and make the deduplication work (concatenating files in CMake! exec(open(...).read())
in Python), I'm very open to hearing alternative approaches. gdb
has its own command file syntax that could be used to load the individual files one after the other.
Considering the horrible things I have to do to try and make the deduplication work (concatenating files in CMake!
exec(open(...).read())
in Python), I'm very open to hearing alternative approaches.gdb
has its own command file syntax that could be used to load the individual files one after the other.
The gdb approach seems reasonable. Perhaps we could create some documentation on debugging and include a gdb helper script to illustrate how to load/use these.
I think this is in a good state for a review now. Sadly, since rmm
is being installed by default inside rapids-compose
, CPM picks up the installed rmm
and thus the load-pretty-printers
file doesn't get created there.
style check cmake-format.............................................................Failed
yes, this depends on https://github.com/rapidsai/rmm/pull/1088 being merged, so I removed the incorrect label. I'll update it soon™️
@gpucibot merge