testutils/floatcmp: revert to base-10 normalization in FloatsMatch #107490
Conversation
|
This change is |
There was a problem hiding this comment.
Thanks for the quick fix!
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @mgartner and @michae2)
-- commits line 6 at r1:
Could you add the failure case to TestFloatsMatch?
In cockroachdb#106552 we tried changing float normalization to use base 2 instead of base 10 (in other words, to use `math.Frexp` instead of our hand-rolled `normalize`). This appears to have broken a logic test, so revert back to the pre-existing base 10 normalization. Fixes: cockroachdb#107461 Release note: None
There was a problem hiding this comment.
You're welcome! Sorry I made the suggestion in the first place. 😝
bors r=rharding6373 p=99
Reviewable status:
Previously, rharding6373 (Rachael Harding) wrote…
Could you add the failure case to TestFloatsMatch?
Done.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status:
|
Build succeeded: |
In #106552 we tried changing float normalization to use base 2 instead of base 10 (in other words, to use
math.Frexpinstead of our hand-rollednormalize). This appears to have broken a logic test, so revert back to the pre-existing base 10 normalization.Fixes: #107461
Release note: None