Skip to content

Fix flexcomp instability when parent body has non-identity quaternion - #3379

Closed
devansh0703 wants to merge 5 commits into
google-deepmind:mainfrom
devansh0703:fix/issue-3364-flexcomp-parent-rotation
Closed

devansh0703 wants to merge 5 commits into
google-deepmind:mainfrom
devansh0703:fix/issue-3364-flexcomp-parent-rotation

Conversation

@devansh0703

Copy link
Copy Markdown
Contributor

Fast path in mj_flexPassiveInterp, mj_flexPassiveBendInterp, and mj_flexPassiveStretch assumed body slide joints are world-aligned. When parent body has a non-identity quaternion, joint axes are rotated, causing wrong force mapping and instability (NaN/Inf in QACC).

Fix: project world-frame forces onto body's local frame via mju_mulMatTVec3(R_body^T, force) before adding to qfrc_spring/damper.

Fixes #3364

Fast path for elastic/damper forces assumed body slide joints are
world-aligned. When parent body has non-identity quaternion, joint
axes are rotated, causing wrong force mapping and NaN in QACC.

Fix: project world-frame forces onto body's local frame via
mju_mulMatTVec3(R^T, force) before adding to qfrc_spring/damper.
@quagla

quagla commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR! Could you please add a test in engine_passive_test that would have caught the bug?

The fast path in mjd_flexInterp_kernel assumed J=I for centered flexes
with all-simple-slider bodies, skipping Jacobian construction and using
K_rot directly. This is incorrect when the parent body has a rotational
offset (R_body != I), producing wrong tangent stiffness and causing
implicit integration to diverge.

Also add TrilinearParentBodyRotation regression test that checks:
- restoring force sign for small perturbations
- rotational invariance of qfrc_passive
- 200-step implicit integration stability
@devansh0703

Copy link
Copy Markdown
Contributor Author

Thanks for the PR! Could you please add a test in engine_passive_test that would have caught the bug?

Done. Added TrilinearParentBodyRotation in engine_passive_test.cc — checks: (1) restoring force sign, (2) rotated vs non-rotated qfrc_passive match within 1e-12, (3) 200-step implicit integration stays stable.

Also found and removed a second instance of the same J=I assumption: the use_fast_path code path in the derivative kernel (mjd_flexInterp_kernel). It skipped Jacobian construction for centered flexes and applied K_rot directly, which is wrong when R_body ≠ I. The test catches this too — the simulation blows up at step 9 without the fix.

Comment thread src/engine/engine_derivative.c Outdated
J_val[current_adr] = blk_jac[r*chain_nnz + idx];
current_adr++;
}
// ponytail: always use general path with correct Jacobian construction.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please restore the fast path and use the rotation in it? It will looks like

if (use_fast_path) {
// apply R to vec
// old code
// apply R^T to res
}

@devansh0703

Copy link
Copy Markdown
Contributor Author

Can you please restore the fast path and use the rotation in it? It will looks like

if (use_fast_path) {
// apply R to vec
// old code
// apply R^T to res
}

Restored the fast path with the rotation correction. Now applies R_body to vec before the K_rot scatter and R_body^T to the result, so it correctly computes R^T * K_rot * R * vec instead of assuming J=I.

Comment thread src/engine/engine_derivative.c Outdated
for (int n = 0; n < npe; n++) {
int dof = m->body_dofadr[bodyid[gindices[n]]];
mju_mulMatVec3(fast_tmp + 3*n, d->xmat + 9*bodyid[gindices[n]],
vec + dof);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this fit in 100 characters (single line).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@devansh0703 could you please just fix this formatting issue and then I will merge your contribution.

Restore the use_fast_path check for centered flexes with simple slider
bodies. Instead of removing the fast path (which assumed J=I), fix it
by applying R_body to vec before the K_rot scatter and R_body^T to the
result, matching the J=R_body Jacobian when the parent body is rotated.

This preserves the performance optimization while fixing the
correctness issue when R_body != I.
@devansh0703
devansh0703 force-pushed the fix/issue-3364-flexcomp-parent-rotation branch from cc24d0e to 3bb874c Compare July 20, 2026 23:18
@quagla

quagla commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

There are failing tests now.

copybara-service Bot pushed a commit that referenced this pull request Jul 23, 2026
Fast path in mj_flexPassiveInterp, mj_flexPassiveBendInterp, and
mj_flexPassiveStretch assumed body slide joints are world-aligned
(J = I). When parent body has a non-identity quaternion, joint axes
are rotated (J = R_body), causing wrong force mapping and instability
(NaN/Inf in QACC).

Fix: project world-frame forces onto body local frame via
mju_mulMatTVec3(R_body^T, force) before adding to qfrc_spring/damper.
Also fix the derivative paths:
- mjd_flexInterp_kernel fast path: R^T * K_rot * R * vec
- mjd_flexStiff_assemble (CSR): R_bi^T * K_rot_block * R_bj

The CSR-assembled stiffness matrix is the actual path used by the
implicit CG solver (mj_flexCG gate). The test uses solver="CG" to
activate this path; without it, flex stiffness is integrated
explicitly and no derivative fix can help.

Ported from GitHub PR #3379
Original author: Devansh (https://github.com/devansh0703)
Fixes #3364

PiperOrigin-RevId: 952789284
Change-Id: If924f7160dd16c0cc88170d80e6e605da0fe2e04
@quagla

quagla commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

I fixed the remaining issues in fe9dc58 so I close this PR, thanks for your work!

@quagla quagla closed this Jul 23, 2026
@devansh0703
devansh0703 deleted the fix/issue-3364-flexcomp-parent-rotation branch September 8, 2026 02:06
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.

flex unstable,Severe numerical instability (NaN/Inf in QACC) of MuJoCo flexcomp when parent body has non-identity quaternion orientation

3 participants