Treat negative zero as equivalent to positive zero in sm90_sparse_gemm_compressor.hpp #2110
Conversation
Signed-off-by: Tyler Michael Smith <[email protected]>
|
@sklevtsov-nvidia , could you please review it? |
include/cutlass/transform/kernel/sm90_sparse_gemm_compressor.hpp
Outdated
Show resolved
Hide resolved
|
|
||
| // Default case - no negative zero | ||
| template <typename T> | ||
| struct has_negative_zero : std::false_type {}; |
There was a problem hiding this comment.
@hwu36 to decide if we want a new file for this trait or if it fits in numeric_types.h or elsewhere
Signed-off-by: Tyler Michael Smith <[email protected]>
| struct has_negative_zero : std::false_type {}; | ||
|
|
||
| // Float types that support negative zero | ||
| // Note that this is false for float8_e4m3_t and float8_e5m2_t |
There was a problem hiding this comment.
Shouldn't it be true, otherwise how is 0x80 value interpreted for these data types? According to the paper it's -0.0, see table 1.
There was a problem hiding this comment.
@alexsamardzic you are correct, we need to add this for all of fp8/fp6/fp4. @tlrmchlsmth feel free to do this if you can, otherwise we can fix this in #2122
There was a problem hiding this comment.
Here is a patch to apply on top of this PR, with FP8 and MX data types added (please check that all of them are there, and make sense here), and also minor changes from my #2122 that may be worth adding. For test/unit/transform/device/sm90_sparse_gemm_compressor_legacy.hpp, I don't think it would work with sub-byte data types, but at the moment I don't have access to hardware to test - so I've put there simpler check for negative zero, but also a static assert that would fail if a sub-byte data type used. I hope we can put the patch into this PR, and I'm closing mine now.
There was a problem hiding this comment.
ah, you're right - I actually got confused between float8_e4m3fnuz and float8_e4m3fn on this one. I'll apply the patch, thanks!
There was a problem hiding this comment.
@alexsamardzic thanks for the patch. I've applied it and it looks good to me. PTAL!
Signed-off-by: Tyler Michael Smith <[email protected]>
This PR handles negative zero in
sm90_sparse_gemm_compressor.hpp, treating it as equivalent to positive zero. This is easy to solve by the caller before calling the compressor however it involves an extra pass of reading and writing the uncompressed sparse matrix, and it would be nice to generally remove this as a potential footgun.