-
Notifications
You must be signed in to change notification settings - Fork 37
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
revert #449 #451
revert #449 #451
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #451 +/- ##
==========================================
- Coverage 70.28% 70.17% -0.11%
==========================================
Files 11 11
Lines 562 560 -2
==========================================
- Hits 395 393 -2
Misses 167 167 ☔ View full report in Codecov by Sentry. |
@@ -21,10 +21,6 @@ ArrayInterface.can_setindex(::Type{<:StaticArray}) = false | |||
ArrayInterface.can_setindex(::Type{<:MArray}) = true | |||
ArrayInterface.buffer(A::Union{SArray, MArray}) = getfield(A, :data) | |||
|
|||
function ArrayInterface.lu_instance(A::SMatrix{N,N}) where {N} |
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.
why was this removed?
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.
because StaticArrays.LU
(which is not the same as LinearAlgebra.LU
) is defined in StaticArrays
not StaticArraysCore
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.
Didn't we need this for performance? What about having a StaticArraysExt for just this one?
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.
I tried that locally and it causes invalidations during precompile which is illegal. I'll just hack my way around it...
Since the SArray types are created in StaticArraysCore, unfortunately we need the extension to be on the library or else generated functions in 3rd party packages can miss the added methods. Furthermore, this breaks the ability to optimize
lu
sinceStaticArraysCore
doesn't defineStaticArrays.LU
so this just is really annoying. I really think we should delete StaticArraysCore and just make it an alias for StaticArrays now that extension packages exist.