Skip to content

Conversation

@stevengj
Copy link
Member

@stevengj stevengj commented Jul 2, 2025

As suggested by @andreasnoack in #1387 (comment), this changes the pinv(::SVD) function to instead return the SVD of the pseudo-inverse, rather than an explicit matrix, so that it can be applied stably.

(No backwards-compatibility issue since the pinv(::SVD) method was introduced in #1387.)

@stevengj
Copy link
Member Author

stevengj commented Jul 2, 2025

Whoops, failing because *(::SVD, ::AbstractVector) is not defined. This should be fixed — otherwise returning an SVD object from pinv is useless.

Update: fixed.

@codecov
Copy link

codecov bot commented Jul 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.85%. Comparing base (9e2ed1c) to head (b3921af).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1398      +/-   ##
==========================================
- Coverage   93.85%   93.85%   -0.01%     
==========================================
  Files          34       34              
  Lines       15827    15830       +3     
==========================================
+ Hits        14855    14857       +2     
- Misses        972      973       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@stevengj
Copy link
Member Author

stevengj commented Jul 4, 2025

CI failure on Windows is an unrelated server glitch.

@stevengj
Copy link
Member Author

stevengj commented Jul 7, 2025

Yay, green again.

@stevengj stevengj merged commit 2c3fe9b into master Jul 7, 2025
4 checks passed
@stevengj stevengj deleted the lazy_svd_pinv branch July 7, 2025 15:10
@MasonProtter
Copy link
Contributor

Maybe a dumb question, but why not also have inv(::SVD) return an SVD?

@dkarrasch
Copy link
Member

We have an open issue (#635) regarding whether inv of factorizations should return a Factorization. The issue is that not every factorization has an inverse that can be represented by a type that we already have. SVD is tempting, of course, but I guess consistency is a potential issue here.

@stevengj
Copy link
Member Author

stevengj commented Jul 12, 2025

@MasonProtter, the other issue is that changing inv(::SVD) would be a breaking change, whereas pinv(::SVD) was a new method (added a few days ago).

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

Successfully merging this pull request may close these issues.

3 participants