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

f32::MAX_EXP and f64::MAX_EXP are documented incorrectly, and other associated constant woes #88734

Open
orlp opened this issue Sep 7, 2021 · 4 comments
Labels
A-floating-point Area: Floating point numbers and arithmetic T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@orlp
Copy link
Contributor

orlp commented Sep 7, 2021

f32::MAX_EXP and f64::MAX_EXP are documented as

Maximum possible power of 2 exponent.

This is straight up not true. They are respectively defined as 128 and 1024, which is actually one above the maximum possible exponent of f32 and f64. The doc comment needs to be changed.


In general I feel the set of associated constants for the floating point types are questionable, and should be a candidate for deprecation and replacement by a better set of constants. It is painfully obvious that the constants were copied from C's <float.h>, with little regard of whether these constants are useful or well-named.

I have no qualms with MIN, MAX, NAN, INFINITY, and NEG_INFINITY at all. They are sane and useful. However, the following set of constants are carbon copied from <float.h>:

FLT_RADIX       = 2                => f32::RADIX
FLT_MIN         = 1.175494e-38     => f32::MIN_POSITIVE
FLT_EPSILON     = 1.192093e-07     => f32::EPSILON
FLT_DIG         = 6                => f32::DIGITS
FLT_MANT_DIG    = 24               => f32::MANTISSA_DIGITS
FLT_MIN_EXP     = -125             => f32::MIN_EXP
FLT_MIN_10_EXP  = -37              => f32::MIN_10_EXP
FLT_MAX_EXP     = 128              => f32::MAX_EXP
FLT_MAX_10_EXP  = 38               => f32::MAX_10_EXP

Going over them one by one (f64 is entirely analogous):

  • f32::RADIX is just plain useless. It's always 2, Rust has no support for non-binary floating point.
  • f32::MIN_POSITIVE is badly named, because it's actually the smallest positive normal number. This is a useful constant, but the name is unacceptable in my opinion.
  • f32::EPSILON is somewhat badly named (MACHINE_EPSILON would be better), and slightly deceptive. However this is not necessarily the fault of the constant, but due to people misunderstanding what machine epsilon means. Should my RFC for next_up/next_down get merged, this would make this constant unnecessary. Especially if we make a ulp method in the future.
  • f32::DIGITS is "the approximate number of significant digits in base 10". I don't know when you'd ever need this constant, or what 'approximate' here means at all. The constant is also deceptive, because one might interpret this as an upper bound on the number of digits needed to represent a f32.
  • f32::MANTISSA_DIGITS includes the implied 1. Thus it is off by one from the constant you almost always want when explicitly working with a mantissa in code: the number of bits that the mantissa is wide.
  • f32::MIN_EXP... I think the doc comment speaks for itself: "One greater than the minimum possible normal power of 2 exponent.". Not only is it off by one, it also ignores denormal floats.
  • f32::MAX_EXP, see start of this issue.
  • f32::MIN_10_EXP, also ignores denormal floats.
  • f32::MAX_10_EXP, sanely defined but also fairly useless since you can compute f32::MAX.log10().floor() if
    you really wanted to know this.

I honestly believe the best way forward is to deprecate all of the above constants and replace them with a couple fundamental, sane and conservative constants. For example for f32:

const EXPONENT_BIAS: i32 = 127;
const EXPONENT_WIDTH: i32 = 8;
const MANTISSA_WIDTH: i32 = 23;
const MIN_POS_NORMAL: f32 = f32::from_bits(1 << f32::MANTISSA_WIDTH);
@camelid camelid added A-floating-point Area: Floating point numbers and arithmetic T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Sep 7, 2021
@hellow554
Copy link
Contributor

I really love your points. Here's one point that one should consider:

People coming from other languages and are used to the terms will search for them. If they don't find them, they get frustrated. So at least the doc should have an alias or state somewhere that this is the alternative to XXX.

@tspiteri
Copy link
Contributor

tspiteri commented Sep 9, 2021

The C standard library follows the convention that floating-point numbers x × 2exp have 0.5 ≤ x < 1, while the IEEE 754 standard text uses the convention 1 ≤ x < 2. This convention in C is not just used for DBL_MAX_EXP, but also for functions such as frexp. f64::MAX_EXP is equal to C DBL_MAX_EXP, which is the reason for this off by one.

@orlp
Copy link
Contributor Author

orlp commented Sep 9, 2021

@tspiteri Thanks for the context, that does explain why those values are the way they are in C. That said, as far as I know Rust doesn't explicitly adopt that convention anywhere, and I don't believe that it should. Rust specifically chose IEEE 754 as the definition of its floating point type, so I believe we're best served to use their conventions and terminology where possible.

@tgross35
Copy link
Contributor

tgross35 commented Jul 3, 2024

@orlp would you consider opening an ACP with what you proposed here? I think the exact changes could probably be negotiated a bit, but the expectation vs. reality of these constants is probably something that we should address, if we even find them useful enough to keep around.

(If you do this, mind suggesting the addition of BITS with it? To mirror the integer types).

Two others issues cover this problem, yours just seems to have the best way forward:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-floating-point Area: Floating point numbers and arithmetic T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants