Static fixes #45
Static fixes #45
Conversation
This extends the `Static` definitions by specializing specifically on array types.
This extends the `Static` definitions by specializing specifically on array types.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #45 +/- ##
=======================================
Coverage 88.45% 88.46%
=======================================
Files 11 11
Lines 1317 1318 +1
=======================================
+ Hits 1165 1166 +1
Misses 152 152 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@devmotion, this unifies static_first, static_step, and static_last with the definitions in Static. I'm not seeing any warnings in the LoopVectorization tests now. |
| @inline Static.static_first(x::AbstractArray) = Static.maybe_static(known_first, first, x) | ||
| @inline Static.static_last(x::AbstractArray) = Static.maybe_static(known_last, last, x) | ||
| @inline Static.static_step(x::AbstractArray) = Static.maybe_static(known_step, step, x) |
There was a problem hiding this comment.
This is type piracy - neither method nor argument is owned by StaticArrayInterface. I think this is not a good fix.
There was a problem hiding this comment.
Good point. Why does it exist in Array interface at all? It's a static thing?
There was a problem hiding this comment.
This is type piracy - neither method nor argument is owned by StaticArrayInterface. I think this is not a good fix.
True but the alternative may be more involved.
Good point. Why does it exist in Array interface at all? It's a static thing?
The alternative solution is to move these known_... methods to Static. I'm fine with doing this but it might take some extra finesse. I'll try moving them to Static.
There was a problem hiding this comment.
Maybe it's possible to remove these definitions here completely and just use the Static versions?
There was a problem hiding this comment.
After refamiliarizing myself with this code I think we have a couple options:
- Accept that
StaticArrayInterfaceis the sole package that gets to do this type piracy because it is specifically for arrays - Move all the
known_<...>methods toStaticand haveStaticdepend onArrayInterface. We originally wantedStaticto be a fairly light dependency, so moving all static related things there wasn't the go to. However, that may not be realistic since Base Julia would ultimately need to change first. - Move
known_<...>methods toArrayInterfacesince those methods don't requireStaticInt. Then definestatic_<...>methods inStatic. Basically, same approach as 2 but different execution. This would take a lot more work to disentangle things but could prepare us to eventually not be so dependent onStatic. I'm not sure how we can completely remove it as a dependency, but if invalidations aren't every going to get fixed I'm not sure we have a choice.
There was a problem hiding this comment.
What are the packages using this?
There was a problem hiding this comment.
When searching for static_first on JuliaHub, it seems only Static.jl uses Static.static_first but all other packages use StaticArrayInterface.static_first: https://juliahub.com/ui/Search?q=static_first&type=code
Maybe the Static methods should just not be exported?
There was a problem hiding this comment.
When searching for
static_firston JuliaHub, it seems only Static.jl usesStatic.static_firstbut all other packages useStaticArrayInterface.static_first: https://juliahub.com/ui/Search?q=static_first&type=codeMaybe the Static methods should just not be exported?
I'd be fine with that or do we want to push for static stuff to be in Static.jl?
There was a problem hiding this comment.
Either export or move to Static.jl, but I don't think anyone actually cares or uses this.
3 participants
No description provided.