Add .npy support to debug_to_file() #8177
Conversation
The .npy format is NumPy's native format for storing multidimensional arrays (aka tensors/buffers). Being able to load/save in this format makes it (potentially) a lot easier to interchange data with the Python ecosystem, as well as providing a file format that support floating-point data more robustly than any of the others that we current support. This adds load/save support for a useful subset: - We support the int/uint/float types common in Halide (except for f16/bf16 for now) - We don't support reading or writing files that are in `fortran_order` - We don't support any object/struct/etc files, only numeric primitives - We only support loading files that are in the host's endianness (typically little-endian) Note that at present this doesn't support f16 / bf16 formats, but that could likely be added with minimal difficulty. The tricky bit of this is that the reading code has to parse a (limited) Python dict in text form. Please review that part carefully. TODO: we could probably add this as an option for `debug_to_file()` without too much pain in a followup PR.
Built on top of #8175, this adds .npy as an option. This is actually pretty great because it's easy to do something like ``` ss = numpy.load("my_file.npy") print(ss) ``` in Python and get nicely-formatted output, which can sometimes be a lot easier for debugging that inserting lots of print() statements (see #8176) Did a drive-by change to the correctness test to use this format instead of .mat.
| char *dst = dict_string_buf; | ||
| char *end = dict_string_buf + kMaxDictStringSize - 1; | ||
|
|
||
| dst = halide_string_to_string(dst, end, "{'descr': '"); |
There was a problem hiding this comment.
Why not a StackStringStreamPrinter here?
There was a problem hiding this comment.
...because I didn't think of it?
|
This function shouldn't call halide_error. Halide_error is called at the Halide IR level if it fails, with a more informative error message (it names the Func) |
|
Would be nice to in some way distinguish in the type system which parts of the runtime should return an error code and have already called halide_error, vs which parts return an error code that still needs a call to halide_error to happen above. |
Yeah, everything about error handling in the runtime is messy. A few years ago I started work on a proposal to rationalize it but ended up abandoning it. |
Surely you don't mean that we shouldn't call it anywhere in halide_debug_to_file()? |
|
PTAL |
|
I do mean that. It produces an error message with less information than if we just return an error code without calling halide_error, and if halide_error has been overridden to return instead of aborting. It produces two error messages for the same error. |
|
PTAL -- I can use StackStringPrinter if you prefer but the underlying codegen will likely be identical, LMK |
| @@ -42,6 +42,8 @@ class DebugToFile : public IRMutator { | |||
| num_elements *= bound.extent; | |||
| } | |||
|
|
|||
| // TODO: why do we bother with this? halide_debug_to_file() | |||
There was a problem hiding this comment.
This code predates halide_buffer_t, and buffer_t only had an elem_size, not a type code. Feel free to fix.
There was a problem hiding this comment.
This code predates halide_buffer_t, and buffer_t only had an elem_size, not a type code. Feel free to fix.
I'll do that as a followup.
|
Gentle Review Ping |
Built on top of #8175, this adds .npy as an option. This is actually pretty great because it's easy to do something like
in Python and get nicely-formatted output, which can sometimes be a lot easier for debugging that inserting lots of print() statements (see #8176)
Did a drive-by change to the correctness test to use this format instead of .mat.