Skip to content

Fix NaN resulting from non-clamped input to simd_asin in angular motor solver#840

Merged
sebcrozet merged 4 commits intodimforge:masterfrom
ryanhcode:fix_angular_motor_nan
Jul 11, 2025
Merged

Fix NaN resulting from non-clamped input to simd_asin in angular motor solver#840
sebcrozet merged 4 commits intodimforge:masterfrom
ryanhcode:fix_angular_motor_nan

Conversation

@ryanhcode
Copy link
Contributor

@ryanhcode ryanhcode commented Jun 5, 2025

Fixes a significant NaN issue caused by a non-clamped input to simd_asin in joint_constraint_builder.rs.

Beforehand, as seen below, slight imprecision in the imaginary components of the error quaternion could cause a value out of bounds to be passed into simd_asin, causing a NaN that further propagates to the body. This occurs very frequently with moving nested revolute joints.

Logged data from the code during debugging showcasing the issue:

ang error [1.0000001, -1.6205013e-5, -1.5124679e-6, 9.6172094e-5]
ang dist NaN

@ThierryBerger
Copy link
Contributor

Thanks for the report! Do you have a reproduction test for this ?

@ryanhcode
Copy link
Contributor Author

@Vrixyz

Angular error is set from let mut ang_err = frame1.rotation.inverse() * frame2.rotation; currently, and then computed from the asin of one of the imaginary components. This evidently can cause the input to be slightly out of the -1.0 to 1.0 range, and therefore produce a NaN.

I do not have a reproduction test for this exact scenario. I could work on creating one if necessary, but I'm uncertain how specific of a test is needed for an issue of this nature.

@ryanhcode
Copy link
Contributor Author

Should be good now.

@ThierryBerger ThierryBerger requested a review from sebcrozet July 8, 2025 13:10
@sebcrozet sebcrozet merged commit 86a257d into dimforge:master Jul 11, 2025
8 checks passed
@sebcrozet
Copy link
Member

Thank you!

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.

3 participants