Ratio::<T>::from_...() - definition of being representable?

[Enyium]

I use this crate to convert an f64 into an fps value (video framerate) consisting of u32 numerator and denominator. The documentation for Ratio::<u32>::from_f64(), for Ratio<T> and for num_traits::cast::FromPrimitive is not clear enough about what exactly it means that the input value can't "be represented by" a ratio.

I read your crate's approximate_float_unsigned() source code and, based on it, wrote the following in the doc comment of my crate's Fps::from_f64():

Returns None in the following cases (according to approximate_float_unsigned() of num-rational crate):

• value < 0.0
• value.is_nan()
• value > u32::MAX as f64, overextending the numerator
• The conversion yields a denominator of 0.

In other cases than mine like Ratio::<u64>::from_f32(), your function's line let t_max_f = <F as NumCast>::from(t_max.clone())?; may also return None.

I'd like to be able to be forever sure about when exactly None can be returned without the necessity of "according to".

Questions/suggestions:

1. Can a denominator of 0 exclusively be yielded when the input f64 is positive/negative infinity?
2. Would it be possible to clarify in your documentation when exactly None can be returned? (Possibly in the documentation for Ratio<T> with references to it in the other locations.)

[cuviper]

1. Can a denominator of 0 exclusively be yielded when the input f64 is positive/negative infinity?

No, this should be None -- it's meant to be an invariant of the crate that the denominator is never zero. That's not a safety invariant, given the safe new_raw, but there are multiple place that document panicking for zero denominator, and I wouldn't be surprised if more are lurking.

Is there a case of "The conversion yields a denominator of 0." that's not already covered by your other points?

2. Would it be possible to clarify in your documentation when exactly None can be returned? (Possibly in the documentation for Ratio<T> with references to it in the other locations.)

FromPrimitive is not prescriptive about this (for better or worse), but I wouldn't mind a PR to add some documentation for how Ratio handles it. That probably shouldn't be too specific, to allow for future improvements, but at least we shouldn't get any worse at conversions. :)

[Enyium]

1. Can a denominator of 0 exclusively be yielded when the input f64 is positive/negative infinity?

No, this should be None

Yes, I confirmed that the function returns None. I'm trying to find out whether my doc comment sentence "The conversion yields a denominator of 0" can be complemented by mentioning infinity. By "conversion", I mean most of the function's code, before the end of the function catches the 0 and transforms it into None.

In other words: Would it be correct to change the sentence to (None returned if...):

• value is positive or negative infinity, so the inner conversion results in a denominator of 0.

---

2. Would it be possible to clarify in your documentation when exactly None can be returned? (Possibly in the documentation for Ratio<T> with references to it in the other locations.)

FromPrimitive is not prescriptive about this

Couldn't the docs of the from_...() methods refer to possible documentation on the Self type? How could this be done in a short sentence, while it only may be available? Example:

Converts an f64 to return an optional value of this type. If the value cannot be represented by this type, then None is returned (see Self type for details).

[cuviper]

In other words: Would it be correct to change the sentence to (None returned if...):

• value is positive or negative infinity, so the inner conversion results in a denominator of 0.

It doesn't result in a denominator of 0 though -- the inner conversion returns None before it ever gets to the calculation of numerator and denominator. Positive infinity is caught by being greater than the maximum possible numerator, u32::MAX in your case, and negative infinity is caught by being less than 0.

The only reason that floats can define division by zero as +/-infinity is that there is also a difference between +/-0.0, so you can kind of treat it mathematically as "the limit as the denominator approaches zero from above" or "from below." There's no such distinction with integer ratios, which is why I'm resisting your statement. :)

Couldn't the docs of the from_...() methods refer to possible documentation on the Self type?

Sure, it could do that, referring to the individual types or their trait impl for details.

[Enyium]

How can the cases be described where the conversion yields a denominator of 0? As a user of the conversion function, I'd like to know at least somewhat deterministic details, so that I can be sure that it can't happen sporadically.

For num_rational::Ratio's docs, I wrote the following:

Converting to a Ratio<T> yields None in the following cases:

• The input value is out of the bounds of the integer target type, which, in case of a float, includes positive and negative infinity.
• The input value is a float and NaN.
• The conversion results in a denominator of 0 before returning. [CAN HAPPEN WHEN EXACTLY?]

[cuviper]

By inspection, I'm not sure what cases reach that denominator of 0. I think that may just be defensive coding, but it could be interesting to put a panic in there and fuzz-test until you hit it. :)

[Enyium]

I put a println!() into your crate (enhanced trait bounds with + Display):

// Overflow
if d1.is_zero() {
    println!("{}", val);
    return None;
}

(By intermediately putting it before the condition, I confirmed that it wasn't optimized away in the following code.)

Then I ran the following code (see comments):

// Converting every `f32` bit pattern takes me 15-25 minutes in a debug build with a single thread.
for i in 0..=u32::MAX {
    let from_val = unsafe { mem::transmute(i) };

    // Problems:
    // - `4_294_967_300_f32` (5 above `u32::MAX`; float bits equivalent to `0x4f80_0000_u32`)
    Ratio::<u32>::from_f32(from_val);

    // Problems:
    // -  `2_147_483_600_f32` (47 below `i32::MAX`; float bits equivalent to `0x4f00_0000_u32`)
    // - `-2_147_483_600_f32` (48 above `i32::MIN`; float bits equivalent to `0xcf00_0000_u32`)
    // Ratio::<i32>::from_f32(from_val);

    // No problems.
    // Ratio::<i16>::from_f32(from_val);

    // No problems.
    // Ratio::<u32>::from_u32(from_val);
    // Ratio::<i32>::from_u32(from_val);
    // Ratio::<u16>::from_u32(from_val);
    // Ratio::<i16>::from_u32(from_val);
}

for i in unsafe {mem::transmute::<_, u64>((u64::MAX - 100_000_000) as f64) }..=u64::MAX {
    let from_val = unsafe { mem::transmute(i) };

    // Problems (aborted early):
    // - `18446744073709552000_f64` (385 above `u64::MAX`)
    // Ratio::<u64>::from_f64(from_val);
}

for i in unsafe {mem::transmute::<_, u64>((i64::MAX - 100_000_000) as f64) }..=u64::MAX {
    let from_val = unsafe { mem::transmute(i) };

    // Problems:
    // -  `9223372036854776000_f64` (193 above `i64::MAX`)
    // - `-9223372036854776000_f64` (192 below `i64::MIN`)
    Ratio::<i64>::from_f64(from_val);
}

// No problems until early abortion.
for i in unsafe {mem::transmute::<_, u64>((u32::MAX - 100_000_000) as f64) }..=u64::MAX {
    let from_val = unsafe { mem::transmute(i) };
    Ratio::<u32>::from_f64(from_val);
}
for i in unsafe {mem::transmute::<_, u64>((i32::MAX - 100_000_000) as f64) }..=u64::MAX {
    let from_val = unsafe { mem::transmute(i) };
    Ratio::<i32>::from_f64(from_val);
}

(println!() seems to round for humans, because input values causing d1.is_zero() in your code were all printed with trailing zeroes. But the values are on either side of the min/max border. Near the i32::MAX border, a 32-bit float can hold either ...648 or ...520 according to my hex editor, and it was printed with ...600.)

As shown in the code, Ratio::<i32>::from_f32(i32::MAX as f32) yields None. Do we now suppose this is the only constraining case regarding the last returning of None? How can this best be conveyed?

I'm not sure why the other cases aren't caught at the top of the function where the range is checked.

[cuviper]

(println!() seems to round for humans, because input values causing d1.is_zero() in your code were all printed with trailing zeroes. But the values are on either side of the min/max border. Near the i32::MAX border, a 32-bit float can hold either ...648 or ...520 according to my hex editor, and it was printed with ...600.)

Yes, the standard library formatting tries to use the least number of significant digits that will still parse back to the same floating point value. Since we're only concerned with integers here, you can format "{:.0}" to see it in full.

As shown in the code, Ratio::<i32>::from_f32(i32::MAX as f32) yields None.

That's correct, because i32::MAX as f32 rounds up to fit in the available mantissa bits, so that floating point value is no longer in range of an i32.

So I think what you've demonstrated is that the earlier q > t_max_f check doesn't catch these when t_max_f gets rounded, i.e. when there are fewer bits in the mantissa than the integer type. But that's an implementation detail -- it's still a case of "the input value is out of the bounds of the integer target type."

However, the other side around i32::MIN is a bug, handled incorrectly because we try to convert the abs() first and apply negation afterward, but that's 1 greater than i32::MAX. We should probably special-case that in approximate_float, before it gets to approximate_float_unsigned.

[Enyium]

However, the other side around i32::MIN is a bug, handled incorrectly because we try to convert the abs() first and apply negation afterward, but that's 1 greater than i32::MAX. We should probably special-case that in approximate_float, before it gets to approximate_float_unsigned.

I wouldn't work on this.

Do you want a dedicated issue for it?