Fix mx.from_fp8 E4M3FN NaN decode with branchless carry - #4376
Open
saud5150 wants to merge 7 commits into
Open
Conversation
## The Bug - Byte 0x7f decodes to 480.0 instead of +NaN - Byte 0xff decodes to -480.0 instead of -NaN ## The Fix Changed decode from: v = (x & 127) << 7; // Wrong: gives ±480.0 To: v = x & 127; u = (v << 7) | (((v + 1) >> 7) << 14); // Right: gives ±NaN How it works: - 0x7f → v = 0x7f → arithmetic sets NaN bit → +NaN - 0xff → v = 0x7f → arithmetic sets NaN bit → then sign bit negates it → -NaN Same 7 bits, sign handled by existing path. No branches, just arithmetic.
Collaborator
|
Some tests are failing, do you mind take a look? |
nastya236
added
bug
low priority
await response
…stead of actually checking anything.
zcbenz
added
await response
Author
|
Set the NaN correctly but then negated it to flip the sign, which doesn't work. Now I will embed the sign bit directly in the fp16 encoding instead. |
zcbenz
removed
the
await response
Author
I will dig a bit deeper into this and add a fix. |
nastya236
added
the
await response
…e code sets that sign correctly, but the × 256 step swaps the whole NaN for the hardware's canonical one, and IEEE 754 doesn't require otherwise.
zcbenz
added
await verification
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4135
The Bug
The Fix
Changed decode from:
v = (x & 127) << 7; // Wrong: gives ±480.0
To:
v = x & 127; u = (v << 7) | (((v + 1) >> 7) << 14); // Right: gives ±NaN
How it works:
Same 7 bits, sign handled by existing path. No branches, just arithmetic.