Skip to content

Fix spring-damper damping regime scaling - #3522

Open
Vedangalle wants to merge 1 commit into
google-deepmind:mainfrom
Vedangalle:fix/spring-damper-scale-invariance
Open

Fix spring-damper damping regime scaling#3522
Vedangalle wants to merge 1 commit into
google-deepmind:mainfrom
Vedangalle:fix/spring-damper-scale-invariance

Conversation

@Vedangalle

Copy link
Copy Markdown

Summary

  • Scale the characteristic-discriminant tolerance by the larger discriminant term.
  • Add analytical regression coverage for overdamped, critically damped, and underdamped responses.
  • Verify time-unit invariance in all three damping regimes.

Fixes #3521.

Problem

mju_springDamper previously compared Kv^2 - 4*Kp directly with the fixed absolute value mjMINVAL. A consistent time rescaling multiplies the discriminant by the square of the time-scale factor, so this comparison could change the selected damping regime even though the physical trajectory was unchanged.

For one equivalent overdamped pair, the unscaled call returned 0.7946142382307649, while the time-scaled call returned 0.0404276819945128. Both should return 0.7946142382307650.

The updated tolerance is relative to max(Kv^2, 4*abs(Kp)). It therefore scales with the discriminant and retains the near-critical branch without changing the public API.

Validation

  • New spring-damper regression tests: 2/2 passed.
  • Full engine_util_misc_test binary: 66/66 passed.
  • Full CTest suite: 1376/1376 passed.
  • Independent property sweep: 1080 equivalent parameterizations passed across all damping regimes and time scales from 1e-2 through 1e-12; maximum absolute difference was 5.33e-15.

Local environment: macOS, AppleClang 21, CMake 4.4.2, Release build with MUJOCO_HARDEN=ON.

@google-cla

google-cla Bot commented Aug 24, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

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

Successfully merging this pull request may close these issues.

mju_springDamper damping regime depends on time units

1 participant