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

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

Open
MJohnson459 opened this issue Mar 31, 2020 · 6 comments

Comments

@MJohnson459
Copy link

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

MJohnson459 commented Mar 31, 2020

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
Copy link
Member

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

@MJohnson459
Copy link
Author

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
Copy link
Member

sebcrozet commented Apr 2, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants