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

Fix bug in pinv #45009

Merged
merged 9 commits into from
May 2, 2022
Merged

Fix bug in pinv #45009

merged 9 commits into from
May 2, 2022

Conversation

hyrodium
Copy link
Contributor

This PR fixes #44234.

Comment on lines 73 to 77
for _ in 1:100
b = a*randn(n)
x = apinv*b
@test norm(a*x-b)/norm(b) ≈ 0 atol=tol1
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the loop here? Is this intended or something left over from editing? Same below in lines 83-87.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since randn(n) is included, I thought it would be better to verify multiple times rather than just one test.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think just one test should suffice. If we do this everywhere, the tests will take way too long.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I have removed for _ in 1:100.

@ViralBShah ViralBShah added the linear algebra Linear algebra label Apr 18, 2022
@hyrodium
Copy link
Contributor Author

I hope this PR will be backported to 1.6 and 1.7.

@ViralBShah ViralBShah added backport 1.6 Change should be backported to release-1.6 backport 1.7 backport 1.8 Change should be backported to release-1.8 labels Apr 26, 2022
@ViralBShah
Copy link
Member

Added the backport labels. Not sure if another 1.7 release will happen before 1.8 comes out.

@StefanKarpinski StefanKarpinski merged commit b4eb88a into JuliaLang:master May 2, 2022
@hyrodium hyrodium deleted the bugfix/pinv branch May 2, 2022 23:25
@KristofferC KristofferC mentioned this pull request May 16, 2022
45 tasks
KristofferC pushed a commit that referenced this pull request May 16, 2022
(cherry picked from commit b4eb88a)
@KristofferC KristofferC mentioned this pull request May 16, 2022
67 tasks
@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.6 Change should be backported to release-1.6 linear algebra Linear algebra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pinv bug: matrix misdiagnosed as diagonal
6 participants