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

std::f32 declares wrong mantissa length #13297

Closed
malu opened this issue Apr 4, 2014 · 7 comments · Fixed by #13710
Closed

std::f32 declares wrong mantissa length #13297

malu opened this issue Apr 4, 2014 · 7 comments · Fixed by #13710

Comments

@malu
Copy link

malu commented Apr 4, 2014

static pub MANTISSA_DIGITS: uint = 53u;
I guess that should be 24. Or is there some reasoning behind it that I don't see?
As well as std::f32::DIGITS which should probably be 6 instead of 15.

@lifthrasiir
Copy link
Contributor

Ugh, many constants in std::f32 are indeed incorrect. The FIXME suggests these constants are deprecated once CTFE (#11621) lands, and corresponding traits are actually correct though.

@thestinger
Copy link
Contributor

CTFE isn't going to happen any time soon, and may not happen at all. They're not going to be possible to remove for a long time.

@aturon
Copy link
Member

aturon commented Apr 22, 2014

There is a broader problem here: the Float trait defines several methods for various constants that almost follow those in the standard float.h, but:

Float::epsilon says

Returns the smallest positive number that this type can represent

while FLT_EPSILON for float.h says

Minimum int x such that FLT_RADIX**(x-1) is a normalised float

(In particular, Float::epsilon matches FLT_MIN in float.h)

Moreover, Float::digits says

Returns the number of binary digits of exponent that this type supports.

while FLT_DIG for float.h says

This is the number of decimal digits of precision for the float data type. Technically, if p and b are the precision and base (respectively) for the representation, then the decimal precision q is the maximum number of decimal digits such that any floating point number with q base 10 digits can be rounded to a floating point number with p base b digits and back again, without change to the q decimal digits.

Finally, the float types implement the Bounded trait, but provide the minimum positive value for Bounded::min_val. I'm guessing this should be the smallest number above negative infinity. But the Bounded trait does not give specs for min_val.

Does anyone know if these discrepancies, which affect both f32 and f64, are intended? My instinct would be to match float.h, and to fix the original bug by having each of f32 and f64 only supply the constants once (they are currently duplicated).

@bjz, care to weigh in?

@aturon
Copy link
Member

aturon commented Apr 22, 2014

Note: this issue and issue 11537 should be resolved together.

aturon added a commit to aturon/rust that referenced this issue Apr 23, 2014
Some of the constant values in std::f32 were incorrectly copied from
std::f64.  More broadly, both modules defined their constants redundantly
in two places, which is what led to the bug.  Moreover, the specs for
some of the constants were incorrent, even when the values were correct.

Closes rust-lang#13297.  Closes rust-lang#11537.
bors added a commit that referenced this issue Apr 24, 2014
Some of the constant values in std::f32 were incorrectly copied from
std::f64.  More broadly, both modules defined their constants redundantly
in two places, which is what led to the bug.  Moreover, the specs for
some of the constants were incorrect, even when the values were correct.

Closes #13297.  Closes #11537.
aturon added a commit to aturon/rust that referenced this issue Apr 25, 2014
Follow-up on issue rust-lang#13297 and PR rust-lang#13710.  Instead of following the (confusing) C/C++ approach
of using `MIN_VALUE` for the smallest *positive* number, we introduce `MIN_POS_VALUE` (and
in the Float trait, `min_pos_value`) to represent this number.

This patch also removes a few remaining redundantly-defined constants that were missed last
time around.
bors added a commit that referenced this issue Apr 25, 2014
Follow-up on issue #13297 and PR #13710.  Instead of following the (confusing) C/C++ approach
of using `MIN_VALUE` for the smallest *positive* number, we introduce `MIN_POS_VALUE` (and
in the Float trait, `min_pos_value`) to represent this number.

This patch also removes a few remaining redundantly-defined constants that were missed last
time around.
@cgbur
Copy link
Contributor

cgbur commented Aug 22, 2023

Hate to dig up a 9 year old issue, but it seems the wrong mantissa is still present. It is the first result on google when you search for f32 mantissa.

https://doc.rust-lang.org/std/primitive.f32.html#associatedconstant.MANTISSA_DIGITS
https://doc.rust-lang.org/std/f32/constant.MANTISSA_DIGITS.html

Should this be fixed or removed?

@wwylele
Copy link
Contributor

wwylele commented Aug 22, 2023

@cgbur the value has been corrected to 24. Is that wrong?

@cgbur
Copy link
Contributor

cgbur commented Aug 22, 2023

I understand the confusion now; the value includes the implicit bit. I was approaching this from the standpoint of bit-shifting and was surprised by the inclusion. In most writing I've seen, it is described as 23, or 24 including the implicit bit. A brief note in the documentation to clarify this might be helpful in case I’m not the only one who made the mistake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants