-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Introduce AdjointFactorization
not subtyping AbstractMatrix
#46874
Conversation
AdjointFactorization
AdjointFactorization
# This is a bit odd since the return is not a Factorization but it works well in generic code | ||
Factorization{T}(A::Adjoint{<:Any,<:Factorization}) where {T} = | ||
# This no longer looks odd since the return _is_ a Factorization! | ||
Factorization{T}(A::AdjointFactorization) where {T} = | ||
adjoint(Factorization{T}(parent(A))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😉
@nanosoldier |
105fe5d
to
ffd8c79
Compare
I guess if we don't restrict |
@nanosoldier |
Your package evaluation job has completed - possible new issues were detected. A full report can be found here. |
@nanosoldier |
Your package evaluation job has completed - possible new issues were detected. |
I believe the ecosystem is ready for this. Shall we wait until all PRs are merged and run another pkgeval test or can we merge this right away? |
Perhaps this needs a NEWS entry. |
@nanosoldier |
|
@nanosoldier |
Your package evaluation job has completed - possible new issues were detected. |
The remaining failing packages have open PRs, so I think this is finally ready to go? |
AdjointFactorization
AdjointFactorization
not subtyping AbstractMatrix
The only failing test is unrelated. Let's go! |
I have long been bugged by the fact that, on the one hand, you can wrap anything by
Adjoint
, even types that don't subtype toAbstractMatrix
, but once wrapped, they are back in theAbstractMatrix
hierarchy. It turns out that this subtyping basically doesn't lead to any usage of fallbacks, except forsize(::Adjoint{Factorization}, ::Int)
!So, this disentangles that, and even restrictsObviously, this is breaking, but I assume it breaks only a few basic packages, but we'll see in a nanosoldier run.Adjoint
/Transpose
to wrap onlyAbstractVecOrMat
, which is not necessary. Anyway, it shows that it can be done.This is a "companion" PR to #46196 and #46233.