Duration::from_secs_f32 gives wrong results

[manio]

Hello.
I am diagnosing why I've got a wrong duration computed from Duration::from_secs_f32()...

I tried this code:

let val1 = Duration::from_secs_f32(30.0);
println!("{}", format_duration(val1).to_string());

I expected to see an exact 30-second duration.
Instead, I've got this:
"30s 1us 24ns"
which is: 30 seconds and 1024 nanos.

I dig deeper into rust code and I've prepared a sample code which is computing it based on the rust/time.rs source code:

    let secs: f32 = 30.0;
    const NANOS_PER_SEC: u32 = 1_000_000_000;
    let nanos = secs * (NANOS_PER_SEC as f32);
    let nanos = nanos as u128;
    let s = (nanos / (NANOS_PER_SEC as u128)) as u64;
    let nano = (nanos % (NANOS_PER_SEC as u128)) as u32;
    let secs = Duration::new(s, nano);
    println!("sec={}, nanos={}", s, nano);

As the result I've got:
sec=30, nanos=1024

Instead of exact 30 seconds and 0 nanos.

[klensy]

rustc -vV ?

[manio]

rustc -vV
rustc 1.50.0-nightly (f76ecd066 2020-12-15)
binary: rustc
commit-hash: f76ecd0668fcdb289456cdc72a39ad15467cc454
commit-date: 2020-12-15
host: x86_64-unknown-linux-gnu
release: 1.50.0-nightly

[klensy]

Works on current stable and nightly: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=5c01f41664bf7f4c81861033f80b6578

I mean, that this issue exist for that versions.

[manio]

@klensy
yep, I also confirmed on:

rustc 1.58.0-nightly (91b931926 2021-10-23)
binary: rustc
commit-hash: 91b931926fd49fc97d1e39f2b8206abf1d77ce7d
commit-date: 2021-10-23
host: x86_64-unknown-linux-gnu
release: 1.58.0-nightly
LLVM version: 13.0.0

[manio]

What is interesting the function: from_secs_f64 doesn't seem to have this problem...
so a quick workaround would be use it and cast from f32 to f64

[klensy]

Extending(?) f32 to u128 gives this addition in 1024:

rust/library/core/src/time.rs

Line 823 in bdcb528

let nanos = nanos as u128;

[manio]

CC: @newpavlov
CC: @mbartlett21
Adding authors ... guys please take a quick look if you have a time...

[nagisa]

f32 will naturally have trouble representing all possible nanosecond values and so doing calculations in f64 may help here somewhat. However the most appropriate approach would be to decompose the float into the whole part and the fractional part and then operate on the two separately to produce the values necessary to create the Duration.

f*::fract and f*::trunc may work sufficiently well for the purpose.

EDIT: I think we would still need to be careful about how the f32 nanosecond fraction is handled. We cannot simply multiply the number by 1_000_000_000 and then convert to an integer, because f32 is not capable of representing integers of such degree losslessly.

[newpavlov]

We cannot simply multiply the number by 1_000_000_000 and then convert to an integer, because f32 is not capable of representing integers of such degree losslessly.

Small correction: 1_000_000_000 is represented exactly by f32, the problem is with 30_000_000_000. The closest float is 30_000_001_024, which results in the 1024 ns error. For example, Duration::from_secs_f32(32.0); would have produced an exact value.

I would say such loss of precision is expected for from_secs_f32 and if you want exact values, you should use the integer based methods. So I don't think this issue should be marked as a bug.

It could be worthwhile to improve the code by processing the fractional part separately, but note that it will not solve this issue fundamentally. We still have the issue of mapping at most 23 fractional bits into ~30 bits, e.g. calling Duration::from_secs_f32(30.3); would still result in an inexact result.

[nagisa]

1_000_000_000 specifically may be representable exactly, but f32 cannot represent all integers 0..1_000_000_000 exactly. Though maybe its not a huge deal if after trunc we end up roundtripping back to the actual integer in all cases.