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

Redesign of triangular matrix types #9779

Merged
merged 1 commit into from
Jan 16, 2015
Merged

Redesign of triangular matrix types #9779

merged 1 commit into from
Jan 16, 2015

Conversation

andreasnoack
Copy link
Member

Instead of using parameters to specify if a triangular matrix is upper, lower and/or has unit diagonal I have decided to write out the four possibilities as types as suggested some time ago by @StefanKarpinski. The four types are UpperTriangular, LowerTriangular, UpperTriangularUnit, LowerTriangularUnit, and right now I have only exported the first two as the last two are probably less used.

The main reason for this change is that the use of parameters makes it cumbersome to work with the types in a way the compiler understands because the type parameters aren't types (:L, :U, true, false). The new Val type would solve this, but I think it is ugly to write Triangular(A, Val{:L}) compared to simply LowerTriangular and as we don't plan to extend the value beyond :L, :U, true and false, the type parameters give us nothing.

The problems with writing the old Triangular types in a way the compiler understands caused \ to have inferred return type Any for the performance critical case where both left and right hand side has the same BlasFloat elements as reported in #9191. This PR fixes #9191 and supersedes #9196.

cc: @jiahao

@tkelman
Copy link
Contributor

tkelman commented Jan 15, 2015

I think I'd prefer UnitTriangular over TriangularUnit

@andreasnoack
Copy link
Member Author

Yes. That is probably better.

@ViralBShah
Copy link
Member

That does sound better.

…nal specification by parameters. This fixes #9191, type instability of \ for BlasFloat types when no promotion is necessary.
@jiahao
Copy link
Member

jiahao commented Jan 17, 2015

This will break a lot of packages.

Would be very helpful to create an entry in Compat.jl for backward compatibility.

@jiahao jiahao added kind:breaking This change will break code domain:linear algebra Linear algebra labels Jan 17, 2015
@IainNZ
Copy link
Member

IainNZ commented Jan 17, 2015

matrixmorpheus added a commit to matrixmorpheus/julia that referenced this pull request Jan 17, 2015
Some rules can be bent. JuliaLang#9803

Others can be broken. JuliaLang#9779
matrixmorpheus added a commit to matrixmorpheus/julia that referenced this pull request Jan 17, 2015
Some rules can be bent. JuliaLang#9803

Others can be broken. JuliaLang#9779
@jiahao
Copy link
Member

jiahao commented Jan 18, 2015

Formatting error in doc change fixed in e98b817

@davidavdav
Copy link
Contributor

Yep---I'd be all for support in Compat.jl for backward compatibility.

@davidavdav
Copy link
Contributor

@andreasnoack Would it make sense to export AbstractTriangular, this would make compatibility somewhat easier? I think I can resolve the references to Triangular in NamedArrays by replacing them by AbstractTriangular.

Now I have my own compat.jl

if VERSION < v"0.4.0-dev"
    typealias AbstractTriangular Triangular
else
    import Base.LinAlg.AbstractTriangular
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:linear algebra Linear algebra kind:breaking This change will break code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

type stability of backslash
6 participants