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

Replace snapshots with hash-based cross-platform determinism test #555

Merged
merged 7 commits into from
Nov 11, 2024

Conversation

Jondolf
Copy link
Owner

@Jondolf Jondolf commented Nov 11, 2024

Objective

Currently, Avian has a cross-platform determinism test that produces insta snapshots, which are compared between platforms in CI with GitHub Actions.

However, these snapshots are quite large, and always pollute diffs in PRs (such as this one) whenever changes affecting simulation behavior are made. Insta must also be installed to generate these snapshots, making it annoying for new contributors to deal with. Overall, it's a hassle.

The test itself is also very simplistic and only tests collisions.

Instead of using snapshots, we should just run the simulation for a while, and compute a single transform hash based on the position and rotation of all bodies. This is much simpler and more convenient.

Solution

Add a 2D test for cross-platform determinism, simulating pairs of objects constrained via revolute joints falling to the ground. After 500 steps, a transform hash is computed and compared against the expected value. If they don't match, the test will fail with a message indicating that the expected hash should be updated if the change in behavior was intended.

test tests::determinism_2d::cross_platform_determinism_2d ... FAILED

failures:

---- tests::determinism_2d::cross_platform_determinism_2d stdout ----
thread 'tests::determinism_2d::cross_platform_determinism_2d' panicked at crates\avian2d\../../src\tests\determinism_2d.rs:63:5:

Expected hash 0xa1c6b39, found hash 0xa38b5132 instead.
If changes in behavior were expected, update the hash in src/tests/determinism_2d.rs on line 61.

This test is based on Box2D's FallingHinges test and sample.

There is also a new determinism_2d example, which is a visual representation of the test.

2024-11-11.20-21-34.mp4

While making this, I stumbled upon some bugs as well:

  • The max correction for revolute joints is way too small, often causing explosive behavior when it is exceeded.
  • When the PhysicsSchedule runs, Time<Substeps> is only set once the substepping loop starts. Earlier in the schedule, the previous value is used. However, some systems like update_contact_softness might need the substep time before that. Using the previous value can lead to seemingly indeterministic results e.g. when resetting scenes.

I have fixed both of these issues.

When the `PhysicsSchedule` runs, `Time<Substeps>` is only set once the substepping loop starts. Earlier in the schedule, the previous value is used.

However, some systems like `update_contact_softness` might need the substep time before that. Using the previous value can lead to seemingly indeterministic results e.g. when resetting scenes.
@Jondolf Jondolf added C-Testing Improvements or additions to testing C-Examples Improvements or additions to examples A-Scheduling Relates to scheduling or system sets A-Determinism Relates to consistent behavior across runs or platforms labels Nov 11, 2024
@Jondolf Jondolf added this to the 0.2 milestone Nov 11, 2024
@Jondolf Jondolf merged commit 2563e71 into main Nov 11, 2024
4 checks passed
@Jondolf Jondolf deleted the transform-hash-determinism-test branch November 11, 2024 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Determinism Relates to consistent behavior across runs or platforms A-Scheduling Relates to scheduling or system sets C-Examples Improvements or additions to examples C-Testing Improvements or additions to testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant