Creating a UnitQuaternion from a Rotation3 of NaNs hangs forever

[joelsa]

MCVE to reproduce:

use std::f64::NAN;
use nalgebra::{UnitQuaternion, Rotation3};

fn main() {
    let r_br: nalgebra::Matrix3<f64> = nalgebra::Matrix3::new(
        NAN, NAN, NAN,
        NAN, NAN, NAN,
        NAN, NAN, NAN,
    );

    let q =
            UnitQuaternion::from_rotation_matrix(&Rotation3::from_matrix(&r_br)).into_inner();
    println!("q: {q}");
}

[joelsa]

The issue is already inside the creation of the Rotation Matrix, this hangs forever as well:

use std::f64::NAN;
use nalgebra::{UnitQuaternion, Rotation3};

fn main() {
    let r_br: nalgebra::Matrix3<f64> = nalgebra::Matrix3::new(
        NAN, NAN, NAN,
        NAN, NAN, NAN,
        NAN, NAN, NAN,
    );

    let r = &Rotation3::from_matrix(&r_br);
    println!("r: {r}");
}

[PaulXiCao]

Looking at other code and documentation it seems like nalgebra does no input validation. Some functions return Options where appropriate (e.g. if the algorithm can fail even for finite elements), e.g. Cholesky::new(). But none checks if the elements are finite (e.g. via f64::is_finite()). I suppose we do not want a special case for Rotation3::from_matrix nor patch all functions?

[joelsa]

I suppose we do not want a special case for Rotation3::from_matrix nor patch all functions?

Yes, you are right. We absolutely do not want this for obvious performance and compatibility reasons.

This issue primarily serves to remind others that this might be their issue, this was a quite hard to track down bug on my embedded code.

The behavior makes intuitive sense, iterative methods like polar‑decomposition or the torque-based-method implemented here check for convergence and otherwise run indefinitely. Less or Greater Comparison with a NaN always returns false, so it runs forever. Documentation of from_matrix_eps states that it may run forever, Documentation for from_matrix does not. from_matrix_eps also has the max_iter argument as a possible remedy.

I think good steps are:

• Update the documentation to make this more clear to the user
  • Specify that from_matrix in itself is just a convenience wrapper for from_matrix_eps with hard-coded max_iter=0, and identity as the initial guess: https://docs.rs/nalgebra/latest/src/nalgebra/geometry/rotation_specialization.rs.html#716
  • Also make it more clear that from_rotation_matrix, as opposed to from_matrix, uses a robust non-iterative method that does not have this issue
• Include a try_from_matrix-wrapper that adds check as a convenience function and returns None if NaNs are present
• Maybe: Add debug_assert to catch this in debug mode only
• Otherwise let the user handle input validation through .is_nan() or .is_finite()

I think any of the above would make nalgebra easiert to use, take a few mins to implement and save other potentially hours of debugging.

I would be happy to implement some or all of these suggestions, if we can get a consensus on which ones are worth adding.