from_matrix returns the identity on a rotation matrix again
- Dominant language
- Rust
- Stars
- 4.8k
- Forks
- 565
- PR merge metrics
- No merged PRs in 30d
Description
TL;DR: I'm encountering an issue where from_matrix silently returns an identity matrix for a valid rotation matrix. This is dangerous, and I’ve proposed a safer replacement function.
I've noticed that similar issues have been reported multiple times in the past. For example, see this GitHub issue: https://github.com/dimforge/nalgebra/issues/1078.
Below is the matrix I'm working with:
```
Matrix3::new(
-1.0, 0.0, 0.0,
0.0, -0.9849588871002197, -0.17278870940208435,
0.0, -0.17278870940208435, 0.9849588871002197,
);
```
This matrix represents a rotation that can be described by the following Euler angles:
- Roll: -9.95 degrees
- Pitch: 0 degrees
- Yaw: 180 degrees
However, when I use from_matrix, it unexpectedly returns an identity matrix instead of the correct rotation. I find this silent failure to be quite dangerous, as it can lead to unnoticed errors in the application.
I’ve also come across attempts to address related issues with from_matrix_eps, such as this pull request: https://github.com/dimforge/nalgebra/pull/1101. This raises a few questions:
What is the original purpose of from_matrix_eps()?
1. If it is necessary, could it return None when it fails to produce a valid result?
2. Alternatively, I propose removing from_matrix_eps() entirely if it’s not essential.
3. To address this issue in our repository, I’ve written the following replacement code for from_matrix. This function includes explicit checks to ensure the input matrix is a valid rotation matrix and returns an error if it isn’t:
```
// Do not use UnitQuaternion::from_matrix, as it can silently fail.
// See test_new_with_matrix in pose_test.rs for details.
fn rotation_from_matrix(matrix: Matrix3) -> Result, PoseError> {
// Check if the matrix is a valid rotation matrix by verifying orthogonality.
let matrix_times_transpose = matrix * matrix.transpose();
if matrix_times_transpose.abs_diff_ne(&Matrix3::::identity(), 1e-6) {
return Err(PoseError::InvalidRotationMatrix);
}
// Check if the determinant is approximately 1 (indicating a proper rotation).
if (matrix.determinant() - 1.0).abs() > 1e-6 {
return Err(PoseError::InvalidRotationMatrix);
}
// If checks pass, construct and return the quaternion.
Ok(UnitQuaternion::from_rotation_matrix(
&Rotation3::from_matrix_unchecked(matrix),
))
}
```
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.