runtime assert failures regarding incompatible dimension parameters are hard to debug

[dsharlet]

When dim runtime checks fail, they are hard to debug, because which dimension and failing values are not identified.

The tests in this repo define some more informative assert macros like ASSERT_EQ(x, y) that show the values involved in the assertion failure, but this requires runtime string handling, which I'd rather avoid as a dependency (outside of the tests).

[fbleibel-g]

Hit this at https://github.com/dsharlet/array/blob/master/array.h#L135, when assigning an array_ref to another with an incompatible shape. With regards to the logging in this assertion failure, I think at the point that we get to the assert inside constexpr_index, the semantics that we'd like to display are already lost. I would have like to see e.g. "assertion failed: trying to assign extent = 3 to static dim<2>::Extent = 1".

Would you consider a function that runs only in debug mode, e.g. assert_shape_compatible(x, y), that is an assert-ing variant of is_shape_compatible, with additional printouts?

And with regards to logging and string manipulation, how about fprintf(stderr, ...) prior to assert?

[dsharlet]

After googling how assert is defined to behave, I think the thing we should do is add a function assert_shape_compatible as you say. I think the implementation should use std::cerr to write things to stderr, (probably indirectly via another assert_dim_compatible function?) and call std::abort if it is failing. All of this should be in an appropriate #ifndef so it's an inline-able empty function when normal asserts aren't enabled.

What do you think?

[fbleibel-g]

Sounds great, I like this plan. I attempt to define this function and send you a tentative PR, unless you'd like to do it instead.

[jiawen]

Does this work for the CUDA versions? assert should work but is hard to test on GitHub. Getting it to work might need some extern tricks so that when linked for device, it calls the builtin.

Similarly, IIRC, CUDA only has printf, not fprintf so stderr won't work.

[dsharlet]

Good point, I'm not sure. There's a CUDA build test, so it should catch any issues found. Maybe we need to overload assert_shape_compatible for __host__ and __device__, and the __device__ one maybe needs to be simpler, maybe just calling assert(is_shape_comptabiel(…)). Maybe some #ifdef hacks to switch between printf(/fprintf(stderr, would work.

[fbleibel-g]

I created https://github.com/dsharlet/array/pull/64 to sketch what this could look like. I think it's easier to catch the mismatch when constructing/assigning to a shape, or we'd have to do this for the array/array_ref types as well as shape.

[dsharlet]

I think this is fixed now, thanks!