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

Conversion tests + bugfix for Unit{Upper/Lower}Triangular #11204

Merged
merged 1 commit into from
May 9, 2015

Conversation

kshyatt
Copy link
Contributor

@kshyatt kshyatt commented May 8, 2015

Converting from Diagonal to UnitUpperTriangular or UnitLowerTriangular wasn't tested and had bugs. In particular, the Unit...Triangular types have unit diagonals, which wasn't checked for at all.

@tkelman tkelman added the linear algebra Linear algebra label May 9, 2015
@tkelman
Copy link
Contributor

tkelman commented May 9, 2015

Have we mentioned enough times how great these are? You're even chipping away bit by bit at JuliaLang/LinearAlgebra.jl#136 which is really good to see.

I think the unit triangular types don't exist in release-0.3 so the bugfix part of this is not a backport candidate, but correct me if I'm wrong.

tkelman added a commit that referenced this pull request May 9, 2015
Conversion tests + bugfix for Unit{Upper/Lower}Triangular
@tkelman tkelman merged commit 403423c into JuliaLang:master May 9, 2015
@andreasnoack
Copy link
Member

Yes, it is really a great job. The unit triangular matrices also exist in 0.3, but there the unit diagonal is marked by a parameter. Hence, it will probably be a bit cumbersome to backport, but we should try fix the bug anyway.

@kshyatt kshyatt deleted the specialer branch May 9, 2015 01:35
@tkelman
Copy link
Contributor

tkelman commented May 9, 2015

okay it'll probably have to be fixed by hand then, but i'll add the label here until then so it doesn't get forgotten

@andreasnoack
Copy link
Member

The conversion to unit triangular is not defined in 0.3, so there is not a bug there. Unless someone asks for it, I don't think we should add the conversion so I'm removing the backport label.

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.

3 participants