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

Make convert(::Type{<:AbstractTriangular}, A::Diagonal) consistently preserve storage structure #17653

Closed
wants to merge 1 commit into from

Conversation

Sacha0
Copy link
Member

@Sacha0 Sacha0 commented Jul 27, 2016

convert(::Type{LowerTriangular}, A::Diagonal) and convert(::Type{UpperTriangular}, A::Diagonal) preserve Diagonal storage structure, respectively returning LowerTriangular{T,Diagonal{T}} and UpperTriangular{T,Diagonal{T}}. In contrast, convert(::Type{UnitLowerTriangular}, A::Diagonal) and convert(::Type{UnitUpperTriangular}) discard the Diagonal storage structure via a full call, respectively returning UnitLowerTriangular{T,Matrix{T}} and UnitUpperTriangular{T,Matrix{T}}.

This pull request makes the latter (Unit(Lower|Upper)Triangular) methods preserve the Diagonal storage structure, consistent with the former ((Lower|Upper)Triangular) methods. Best!

@tkelman
Copy link
Contributor

tkelman commented Jul 27, 2016

add a test?

@kshyatt kshyatt added linear algebra Linear algebra needs tests Unit tests are required for this change labels Jul 27, 2016
@Sacha0
Copy link
Member Author

Sacha0 commented Jul 27, 2016

add a test?

Good call! Tests added. Thanks!

@kshyatt
Copy link
Contributor

kshyatt commented Jul 27, 2016

Are these tests a good candidate for @inferred?

@tkelman tkelman removed the needs tests Unit tests are required for this change label Jul 27, 2016
@Sacha0
Copy link
Member Author

Sacha0 commented Jul 27, 2016

Are these tests a good candidate for @inferred?

What makes a good candidate for @inferred? Apologies for my ignorance.

@kshyatt
Copy link
Contributor

kshyatt commented Jul 27, 2016

IIUC @inferred tests if the type you get back is the one the compiler expects - it's a good way to check type stability.

@Sacha0
Copy link
Member Author

Sacha0 commented Jul 27, 2016

IIUC @inferred tests if the type you get back is the one the compiler expects - it's a good way to check type stability.

Apologies for not being clear. To rephrase the question, when is testing with @inferred particularly worthwhile (in contrast to instances where testing with @inferred is possible yet not done)? Instances of observed type instability? Thanks!

… Diagonal storage structure, consistent with conversion to (Upper|Lower)Triangular. Test.
@Sacha0
Copy link
Member Author

Sacha0 commented Jul 31, 2016

Superseded by #17723.

@Sacha0 Sacha0 closed this Jul 31, 2016
@Sacha0 Sacha0 deleted the diagconvtri branch August 7, 2016 19:13
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