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

SVD fails to order singular values for 3x3 matrix #1215

Open
rasmusgo opened this issue Mar 4, 2023 · 2 comments
Open

SVD fails to order singular values for 3x3 matrix #1215

rasmusgo opened this issue Mar 4, 2023 · 2 comments

Comments

@rasmusgo
Copy link
Contributor

rasmusgo commented Mar 4, 2023

I had a bug in my physics simulation (not rapier) that I tracked down to relying on svd to provide singular values in sorted order. The doc for svd states:

Computes the Singular Value Decomposition using implicit shift. The singular values are guaranteed to be sorted in descending order. If this order is not required consider using svd_unordered.

But the singular values I was getting were not sorted. Here is a test that reproduces the bug:

#[test]
fn svd3_sorted() {
    let mat = Matrix3::<f32>::new(
        1.0,
        0.0,
        0.0,
        0.0,
        0.99999994,
        0.0,
        0.0,
        -0.00000054691924,
        1.0000005,
    );
    let svd = mat.svd(true, true);
    assert!(
        svd.singular_values[2] <= svd.singular_values[1]
            && svd.singular_values[1] <= svd.singular_values[0],
        "Singular values are not sorted in descending order: {}",
        svd.singular_values
    );
}

This results in:

thread 'svd3_sorted' panicked at 'Singular values are not sorted in descending order: 
  ┌            ┐
  │  1.0000005 │
  │ 0.99999994 │
  │          1 │
  └            ┘

I am using nalgebra version 0.32.1.

@rasmusgo
Copy link
Contributor Author

rasmusgo commented Mar 4, 2023

There is a typo in svd3.rs:45 where rho0 is swapped with rho2 but it should be rho1 that is swapped with rho2. I don't think this is the cause of the bug though.

@rasmusgo
Copy link
Contributor Author

rasmusgo commented Mar 4, 2023

Here is an updated test that looks more like the tests in tests/linalg/svd.rs

#[test]
// Exercises bug reported in issue #1215 of nalgebra (https://github.com/dimforge/nalgebra/issues/1215)
fn svd_regression_issue_1215() {
    let x = nalgebra::matrix![
        1.0f32, 0.0, 0.0;
        0.0, 0.99999994, 0.0;
        0.0, -0.00000054691924, 1.0000005
    ];
    let x_svd = x.svd(true, true);
    assert!(is_sorted_descending(x_svd.singular_values.as_slice()));
}

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

No branches or pull requests

1 participant