svd: add pseudo_inverse computation

[aentity]

I don't know faer codebase or linear algebra well so I try to do my best. please let me know for corrections, and if unsafe is ok. thank you for excellent library

[sarah-ek]

sorry, im in the middle of a big refactor at the moment. so i'd rather not merge pull requests until #107 is merged. but this seems like a nice contribution!

[aentity]

ok @sarah-ek thank you for consideration. please let me know when finished, and i will attempt rebase and fix.

[sarah-ek]

it's merged now. it might take a bit more than a rebase though :sweat_smile:

[aentity]

thank you it was not bad :)) i like addition of ColRef it is nice.

[sarah-ek]

looks mostly good to me. just have a few comments on it

• for SINGLE_VALUE_RCOND: f64 = 1e-15, this should be replaced by E::Real::faer_epsilon()
• it looks like you're doing a custom matrix multiply in compute_pseudo_inverse, it would be better to instead use the library-provided implementation. either using * or crate::linalg::matmul::matmul
• you don't need to take the absolute value of singular values, as they're already guaranteed to be real and nonnegative

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 86.56%. Comparing base (b62d42f) to head (ddba518). Report is 7 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #106 +/- ## ========================================== - Coverage 86.59% 86.56% -0.03% ========================================== Files 115 116 +1 Lines 87234 87361 +127 ========================================== + Hits 75538 75626 +88 - Misses 11696 11735 +39 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[aentity]

hello, i have fixed 1 and 3, thank you. i am unfamiliar with faer to understand how to fix 2. do you suggest to replace:

                    let val = vt.read(k, i).faer_mul(u.read(j, k)).faer_div(sv_k);
                    unsafe { ai.write_unchecked(i, j, ai.read(i, j).faer_add(val)) };

or some other part? thank you

[aentity]

i have pushed two suggestions, thank you, i will await suggestion for how to integrate matrix mul. i do not see way to use if condition.

[sarah-ek]

the idea is to compute V S.+ first, then multiply that by U.T, rather than trying to compute V S.+ U.T all at once

[aentity]

how would addition and multiplication handle the branch?

if sv_k > sv_tolerance {
  // do fused add mult
}

[sarah-ek]

the branch would be done in the V S.+ multiplication, by replacing the entries of S.+ that would be too large by 0

[aentity]

i'm sorry i need more help than suggestion, or if you know quick answer, please post or edit. i have only made this pr because i need to compute pseudo inverse for ambisonic speaker decoder calculations, and pure rust solution is ideal.

[sarah-ek]

in that case, would it be fine if we merged this PR as is, then i applied the fix myself?

[aentity]

in that case, would it be fine if we merged this PR as is, then i applied the fix myself?

yes i would be very happy. only thing is the tests i added, i know they are correct values, so as long as doesn't alter tests, everything is ok. but i think you know much more what is correct and what is expected here than me :))

also i rebased on main and fixed compile error, hopefully tests should compile now.

[sarah-ek]

one last thing, would you be fine with changing the license to MIT? i would prefer to not mix multiple licenses in one project if i can help it

[aentity]

yes i will of course add whatever license you prefer. i just copied license doc comment from jacobi.rs in svd mod which has this license. in most recent push, i simply deleted license header if ok for you.

[sarah-ek]

that's fine, yes. the jacobi.rs header has that license because it was copied from eigen if i recall correctly