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

Allow rank computation for QRPivoted matrices #54283

Merged
merged 6 commits into from
Apr 30, 2024
Merged

Allow rank computation for QRPivoted matrices #54283

merged 6 commits into from
Apr 30, 2024

Conversation

danilonumeroso
Copy link
Contributor

@danilonumeroso danilonumeroso commented Apr 27, 2024

Hi, this fixes JuliaLang/LinearAlgebra.jl#1052. I followed @stevengj's code suggestion which worked just fine and added tests in the related section.

Note: this is my first contribution so please feel free to point out any rookie mistakes I might have made!

@jishnub jishnub added the linear algebra Linear algebra label Apr 27, 2024
@stevengj
Copy link
Member

LGTM. Do you also want to tackle JuliaLang/LinearAlgebra.jl#1052? If nullspace and cond aren't addressed in this PR, which is fine, we should open a new issue for those after this PR is merged.

@stevengj stevengj added the needs news A NEWS entry is required for this change label Apr 27, 2024
@stevengj
Copy link
Member

stevengj commented Apr 27, 2024

Probably worth adding a NEWS.md entry.

Not sure where else it would be good to document this?

@danilonumeroso
Copy link
Contributor Author

LGTM. Do you also want to tackle #53214 (comment)? If nullspace and cond aren't addressed in this PR, which is fine, we should open a new issue for those after this PR is merged.

Yes, I'd like to. I'm going to open a new issue for those and tackle it in a separate PR

Probably worth adding a NEWS.md entry.

Not sure where else it would be good to document this?

Done! Please tell me if I need to be more specific and/or there's any standard I should follow for NEWS.md entries

NEWS.md Outdated Show resolved Hide resolved
@LilithHafner LilithHafner added merge me PR is reviewed. Merge when all tests are passing and removed needs news A NEWS entry is required for this change labels Apr 28, 2024
NEWS.md Outdated Show resolved Hide resolved
@fatteneder fatteneder merged commit 831ebe0 into JuliaLang:master Apr 30, 2024
7 checks passed
@fatteneder fatteneder removed the merge me PR is reviewed. Merge when all tests are passing label Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linear algebra Linear algebra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rank(::QRPivoted) method?
6 participants