Repository navigation
Duration::from_secs_f32 gives wrong results #90225
Description
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.
Activity
klensy commentedon Oct 24, 2021
rustc -vV?manio commentedon Oct 24, 2021
klensy commentedon Oct 24, 2021
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 commentedon Oct 24, 2021
@klensy
yep, I also confirmed on:
manio commentedon Oct 24, 2021
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 commentedon Oct 24, 2021
Extending(?) f32 to u128 gives this addition in 1024:
rust/library/core/src/time.rs
Line 823 in bdcb528
Fix Duration::from_secs_f32
manio commentedon Oct 24, 2021
CC: @newpavlov
CC: @mbartlett21
Adding authors ... guys please take a quick look if you have a time...
nagisa commentedon Oct 24, 2021
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*::fractandf*::truncmay work sufficiently well for the purpose.EDIT: I think we would still need to be careful about how the
f32nanosecond fraction is handled. We cannot simply multiply the number by1_000_000_000and then convert to an integer, becausef32is not capable of representing integers of such degree losslessly.newpavlov commentedon Oct 25, 2021
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_f32and 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 commentedon Oct 25, 2021
1_000_000_000specifically may be representable exactly, but f32 cannot represent all integers0..1_000_000_000exactly. Though maybe its not a huge deal if aftertruncwe end up roundtripping back to the actual integer in all cases.newpavlov commentedon Oct 25, 2021
Argh, I forgot that the
truncmethod is not available oncore, so the linked PR will not work out of box.UPD: I have implemented manual fraction/exponent splitting and their conversion to integers.
Rollup merge of rust-lang#90247 - newpavlov:duration_float_fract, r=n…