Improve determinism with inserting by Entity order

[cscorley]

Looking for feedback on this approach. I understand this likely requires additional changes before acceptance.

I have a physics simulation that, when ran twice side-by-side, can produce slightly different data between the two simulations. I determine this data desync by serializing the RapierContext (using bincode) on each frame and then hashing the result. However, after the very first frame, when the SyncBackend systems get to run for the first time, my two simulations return two different hashes. I have the physics and query systems disabled to start, so it is not from StepSimulation yet. I have "enhanced-determinism" on and have a guaranteed creation order of Entities. I have exactly 100 Entities with both RigidBody and a Collider.

This non-determinism in the hash stems from Bevy not having a guaranteed Entity order for Queries, and I have found it fruitful to enforce that constraint in Rapier's systems. This PR shows how I have handled this in my fork. Doing a sort on the Queries as I have done in this PR seems to ensure that all of Rapier's internal data are initialized in exactly the same order, and thus the hashes returned are always the same.

Open questions I have are:

1. What is the best way to introduce this without also adding performance concerns to users? An option on the RapierConfiguration? There is a feature for "enhanced-determinism", but that has always seemed like a way to select packages to me.

2. Is there a more efficient way to sort these by Entity?

3. I feel sorting by Entity is the best way to accomplish this for my purposes, but are there other ways a user may want/need to sort these?

[derrick56007]

Thanks for looking into this one. I'm currently working on some simulations myself and found differing results from the same builds on different platforms.

For your first point, I would prefer this to be included within the feature for "enhanced-determinism" with the option to turn it off. By doing so, cross-platform determinism would work "out of box" and would prevent further confusion.

[cscorley]

@sebcrozet I am ready for this to be reviewed.

I selected the feature flag for this purpose because it makes the for ... x.iter() simpler to write in Rust without having to worry about move semantics.

I implemented sorts on these queries because they have inserts on variables within the context (entity2body, etc).

[sebcrozet]

These changes are not doing what you expect. For example:

    #[cfg(feature = "enhanced-determinism")]
    {
        let mut impulse_joints: Vec<(Entity, &ImpulseJoint)> = impulse_joints.iter().collect();
        impulse_joints.sort_by_key(|f| f.0);
    }

    for (entity, joint) in impulse_joints.iter() {

Will create a new impluse_joints vec and sort it. Then that vec runs out of scope and is dropped (because of the { ... }), so the impulse_joints.iter() is still iterating through the unsorted query.

[cscorley]

Ah, yes, you are right. I was testing this on a different branch and added all of the feature checks after-the-fact and was not working against the correct branch. My apologies.

Is there much point to putting this behind a feature flag at all?

[cscorley]

Hi @sebcrozet, just wanted to ping you on this PR again. Please let me know if there's anything else I can do to improve this change. Thanks!