dimforge / nphysics

2 and 3-dimensional rigid body physics engine for Rust.
https://nphysics.org
Apache License 2.0
1.62k stars 121 forks source link

The mechanical world stops updating after 524,288 seconds when using `f32` precision #252

Open MJohnson459 opened 4 years ago

MJohnson459 commented 4 years ago

I have created a simulator based on nphysics and I've noticed that after 6 days the mechanical world stops ticking correctly. This seems to be a floating point precision issue regarding the parameters t and dt. Using the default dt = 1.0/60.0 then the simulator stops at exactly 524,288 seconds.

let t = 524288.0f32;
assert_eq!(t + 1.0/60.0, t);

While it would be possible to change the simulator to run on f64 values, nothing apart from the time requires that precision.

MJohnson459 commented 4 years ago

It might make sense to convert the time related values to use std::time structs which should handle a much larger time period. https://doc.rust-lang.org/std/time/struct.Instant.html

sebcrozet commented 4 years ago

Hi! What exactly do you mean by "the mechanical world stops ticking correctly"? What behavior are you observing and what is your setup?

While nphysics does maintain the total elapsed time in the IntegrationParameters::t variable, it is never used for the simulation itself. So even large running times should not affect the simulation itself.

MJohnson459 commented 4 years ago

Ok so I am using IntegrationParameters::t to keep the sim in sync with real time.

let start = time::Instant::now();
...
while mechanical_world.integration_parameters.t < start.elapsed().as_secs_f32() {
    let real_elapsed = start.elapsed().as_secs_f32();
    let sim_elapsed = mechanical_world.integration_parameters.t;

    if real_elapsed - sim_elapsed > 0.5 {
        warn!("Jerk to catch up - real: {:?}, sim: {}", real_elapsed, sim_elapsed );
    }

    mechanical_world.tick(...);
}

So when IntegrationParameters::t stops updating I get stuck in an infinite loop. I could just stop relying on it but it seems like a limitation? Would it be possible to just make the type of t defined as an f64 rather than tied to the rest of the sims type?

sebcrozet commented 4 years ago

Oh, I see. In that case we should change t to use std::time::Duration as you suggested instead.

MJohnson459 commented 4 years ago

I can make a PR. It was pointed out on discord that it might be best to avoid std::time as we might want to keep nphysics free of units. std::time::Duration uses a mix of u64 and u32 to go up to billions of years and down to nanoseconds.

pub struct Duration {
    secs: u64,
    nanos: u32, // Always 0 <= nanos < NANOS_PER_SEC
}

Is this good enough or should it just be changed to an f64?

sebcrozet commented 4 years ago

Mmh, it is right that nphysics is free of units. I also think that using f64 is not viable because the scalar type used by nphysics could very well be greater than 64-bits (because the user could use decimal d128 types once they support more math operations).

So after some thought I think it is best to simply deprecate t and leave it to the user to track the time as he wishes. nphysics itself does not use this t anyway.

Edit: I've seen the discord conversation just now. It appears the removal of t has been suggested by other contributors too.