-
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
NDIndex - Optionally static CartesianIndex #140
Conversation
Previously conversion from an integer to a cartesian index required a call to `axes`, but offsets and the array size are acquired from the array instead. This shouldn't make a difference in most cases, but if an array type has optimized these methods over creation of an entire axis it could be quicker.
Codecov Report
@@ Coverage Diff @@
## master #140 +/- ##
==========================================
+ Coverage 84.17% 84.38% +0.21%
==========================================
Files 10 11 +1
Lines 1460 1531 +71
==========================================
+ Hits 1229 1292 +63
- Misses 231 239 +8
Continue to review full report at Codecov.
|
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.
Seems fine, but I realize now that I VectorizationBase/LoopVectorization shouldn't need this anymore.
Once upon a time, ranges all round-tripped through static_first(r):static_last(r)
, so I needed static CartesianIndex
to preserve statically sized CartesianIndices
.
Now it uses LoopVectorization.canonicalize_range
instead.
It's not necessarily something we absolutely can't live without, but it also gives you the nice guarantee that you have a bunch of |
While working on more indexing stuff I needed the ability to propagate static info through something like
CartesianIndex
. I also saw there was a variant of this sort of thing in VectorizationBase.jl, so I think it could be widely beneficial to start getting something standard in place.