More non-1 indexing fixes #19015
More non-1 indexing fixes #19015
Conversation
| reduced_dims(::Tuple{}, region) = () | ||
| function reduced_dims(dims::Dims, region) | ||
| Base.depwarn("`reduced_dims` is deprecated for Dims-tuples; pass `indices` instead", :reduced_dims) | ||
| map(r->last(r), reduced_dims(map(n->OneTo(n), dims), region)) |
There was a problem hiding this comment.
map(last, reduce_dims(map(OneTo, dims), region))? Shouldn't be necessary to construct anonymous functions in those places, I would think.
| function reduced_dims0{N}(siz::NTuple{N,Int}, region) | ||
| rsiz = [siz...] | ||
| function reduced_dims0{N}(inds::Indices{N}, region) | ||
| rinds = [inds...] |
There was a problem hiding this comment.
Would collect be any more efficient than splatting in this case?
There was a problem hiding this comment.
Since the length of inds is a type parameter, I would say splatting is fine: the compiler is going to specialize on that anyway.
There was a problem hiding this comment.
It turns out that splatting is (barely) more efficient, at least for the couple of Ns I tried.
timholy
force-pushed
the
teh/stats_indices
branch
from
8bc47ee
to
750d93b
Compare
|
probably should have run nanosoldier here before merging |
|
Hmm, you're right. I'll run it locally and see if I see anything. |
|
No real regressions turned up (whew), but I noticed that we currently have disastrous performance on benchmarks of this function, as appearing in e.g. |
|
Looks like the problem is with |
|
I can fix the type instability of |
|
Could fix it and add a |
|
Doesn't seem to be backportable as-is. It touches a lot of code, deprecates several functions and restricts signatures... |
|
See the "backporting guide" in the top post. |
|
Ah, sorry. Then better make a separate PR and remove the label from this one. |
|
See #19100. |
|
Are packages expected to need to implement |
|
Not for |
Contains fixes in
setindex!, reducedims functions, and some statistics functions.Backporting guide: on 0.5, the added deprecations should be moved to
reducedims.jl, and thedepwarnlines deleted. (Can't introduce new deprecations in the middle of a release cycle.)