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

revert #449 #451

Conversation

oscardssmith
Copy link
Contributor

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 since StaticArraysCore doesn't define StaticArrays.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.

Copy link

codecov bot commented Jul 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.17%. Comparing base (50e2776) to head (e520267).
Report is 2 commits behind head on master.

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.
📢 Have feedback on the report? Share it here.

@@ -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}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why was this removed?

Copy link
Contributor Author

@oscardssmith oscardssmith Jul 30, 2024

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

Copy link
Member

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?

Copy link
Contributor Author

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...

@ChrisRackauckas ChrisRackauckas merged commit df0b598 into JuliaArrays:master Jul 30, 2024
17 of 18 checks passed
@oscardssmith oscardssmith deleted the os/partially-revert-refactor-StaticArrays-ext branch July 30, 2024 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants