Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong result of min() and max() in nalgebra::base::min_max::Matrix when f64::NAN is included #1329

Open
yzobus opened this issue Dec 7, 2023 · 4 comments
Labels

Comments

@yzobus
Copy link

yzobus commented Dec 7, 2023

There seems to be a bug for the min() and max() methods of nalgebra::base::min_max::Matrix when an f64::NAN is included in the matrix.
Specifically, the entries before the NAN-entry are discarded when using these functions, which results in a wrong output.

For example in this simplified function:

fn main() {
    let mat_wrong_min = MatrixXx1::from_vec(vec![-3., f64::NAN,-2.,-1.,0.]);
    let mat_wrong_max = MatrixXx1::from_vec(vec![3., f64::NAN,2.,1.,0.]);

    println!("min:\t{}", mat_wrong_min.min());
    println!("min:\t{}", mat_wrong_max.max());
};

The first println! statement prints -2., instead of -3. and the second println! statement prints out 2. instead of 3.

@Andlon
Copy link
Sponsor Collaborator

Andlon commented Dec 7, 2023

While this is definitely a bug, I do not necessarily agree that the first result should give -3, but instead NaN to indicate that the result is not well-defined.

@Andlon Andlon added the bug label Dec 7, 2023
@Andlon
Copy link
Sponsor Collaborator

Andlon commented Dec 7, 2023

@yzobus: can you please indicate the version of nalgebra you're using? Is it the latest one?

@yzobus
Copy link
Author

yzobus commented Dec 7, 2023

@Andlon You are right with your comment. However, I thought maybe this was intended, similar to the methods min_skipnan or max_skipnan of the ndarray crate.

I tested this with version "0.32.3" and "0.30"

@arscisca
Copy link
Contributor

arscisca commented Dec 15, 2023

Both the min() and max() methods use simba crate's SimdPartialOrd methods, which I believe to be the source of the issue. I created the following: dimforge/simba#51.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants