Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

16-bit float support #296

Merged
merged 20 commits into from
Feb 7, 2025
Merged

16-bit float support #296

merged 20 commits into from
Feb 7, 2025

Conversation

dalle
Copy link
Collaborator

@dalle dalle commented Dec 4, 2024

Potential fix for issue #148
Augmenting initial work done in pull request #264

Still some issues remain.

  • One test is failing:
Checking data/ulfjack-ryu.txt
fesetround to FE_UPWARD
bad 16: 0000 33000000 3E60000000000000 2.98023223876953125E-8
parsed as 0x1p-24 (5.9604644775390625e-08)
as raw uint16_t, parsed = 1, expected = 0
fesetround: FE_UPWARD
  • Cleaning of code
  • More tests and cleaning up tests
  • Update readme

@dalle
Copy link
Collaborator Author

dalle commented Dec 4, 2024

I would argue that the test is incorrect, parsing 2.98023223876953125E-8 (0.0000000298023223876953125) is exactly halfway to 5.9604644775390625E-8 (0.0000000596046447753906250) and would round upwards for fesetround: FE_UPWARD.

@lemire
Copy link
Member

lemire commented Dec 4, 2024

@dalle Thanks. I will have a look shortly.

Note that I do not trust the tests that I had included. We need to validate.

@dalle dalle added the help wanted Extra attention is needed label Jan 24, 2025
@dalle
Copy link
Collaborator Author

dalle commented Feb 5, 2025

@lemire Do you know a path forward on this?

@lemire
Copy link
Member

lemire commented Feb 5, 2025

@dalle I was hoping to have a look today (yeah, not kidding).

As you realize, we need to validate the tests carefully and this must be done manually. :-)

@dalle
Copy link
Collaborator Author

dalle commented Feb 5, 2025

@dalle I was hoping to have a look today (yeah, not kidding).
That's amazing 🎉

As you realize, we need to validate the tests carefully and this must be done manually. :-)
I understand it is a complex task 🙏

@lemire
Copy link
Member

lemire commented Feb 6, 2025

Ok. So 5.9604644775390625E-8 is exactly 2**-25. The smallest value (subnormal) that can be represented using float16 is 2**-24. So 5.9604644775390625E-8 is exactly midpoint between the float16 0 and the smallest float16 value. My interpretation of round-to-even is that the result should be zero.

The fesetround: FE_UPWARD is a red herring. I don't think it matters because fast_float tries to be independent from fesetround (since the C++ standard regarding from_chars makes no mention of fesetround, I assume that it means that we don't respect fesetround).

For reference, let us consider float32... the smallest value that can be represented is 2**−149. So how would 2**−150 be represented? That value is 0.000000000000000000000000000000000000000000000700649232162408535461864791644958065640130970938257885878534141944895541342930300743319094181060791015625. We can test it out:

  std::string special_case = "0.000000000000000000000000000000000000000000000700649232162408535461864791644958065640130970938257885878534141944895541342930300743319094181060791015625";
  float parsed;
  fast_float::from_chars(special_case.data(), special_case.data() + special_case.size(), parsed);
  std::cout << std::hexfloat << parsed << std::endl;

We get zero. So I think that the failing test is indeed correct. So there is at least one bug remaining... although it might be the only one!

@lemire
Copy link
Member

lemire commented Feb 6, 2025

So what happens here is that we effectively have a round-to-even case together with a subnormal. This breaks our assumption.

// Thankfully, we can't have both "round-to-even" and subnormals because
// "round-to-even" only occurs for powers close to 0 in the 32-bit and
// and 64-bit case (with no more than 19 digits).

So additional math work is needed.

@lemire
Copy link
Member

lemire commented Feb 7, 2025

I am still merging, but I will open an issue.

@lemire lemire merged commit 7a5ee5a into fastfloat:main Feb 7, 2025
36 of 37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants