precondition checks in extents constructors#410
Conversation
| auto test_precondition_indices_not_representable = [] { | ||
| [[maybe_unused]] auto exts = Kokkos::dextents< std::int8_t, 2 >{ 500, 500 }; | ||
| }; | ||
| EXPECT_DEATH(test_precondition_indices_not_representable(), ""); |
There was a problem hiding this comment.
We do not want to ensure that the error message matches some regex?
There was a problem hiding this comment.
We can at least for CPU but it won't work with CUDA. So I just decided not to since it doesn't matter a whole lot
There was a problem hiding this comment.
What do you mean it won't work with CUDA?
We do check error messages from the device in Kokkos so there is some level of support.
There was a problem hiding this comment.
So for now the CUDA precondition violation handler prints to stdout. EXPECT_DEATH checks stderr -- we could do something like:
#if defined(MDSPAN_IMPL_HAS_CUDA) || defined(MDSPAN_IMPL_HAS_HIP) || defined(MDSPAN_IMPL_HAS_SYCL)
EXPECT_DEATH(test_precondition_indices_not_representable(), "");
#else
EXPECT_DEATH(test_precondition_indices_not_representable(), "extent_is_representable");
#endifWe do this elsewhere. But really the precondition handler doesn't print the best errors anyway
There was a problem hiding this comment.
I updated to test the message on host. On device the message is kinda useless:
void Kokkos::detail::default_precondition_violation_handler(const char*, const char*, unsigned int): Assertion `0' failed.
So I think we should avoid testing device messages for now until we update our precondition violation handler on device (likely a separate PR)
There was a problem hiding this comment.
Oh, I see, because we call printf(...) and assert(false) instead of __assert_fail(...) like we do in Kokkos
…ionally modify in_range implementation to static assert the types are integral
290c211 to
8cb9a2c
Compare
This adds the missing precondition checks for mdspan.extents.cons