[JLArrays] allow mapreducedim! into wrapper - #754
Conversation
Previously, e.g. `sum!(transpose(v), M)` would error
|
Hmm, this is annoying! Looks like all |
Discovered in JuliaGPU/GPUArrays.jl#754. Once this is approved, I will open PRs for all the other backends as well.
`mapreducedim!` reshapes the output container to match the number of dimensions of the input, but then returned that reshaped array instead of the one it was handed. Keep a reference to the original and return that. Same as JuliaGPU/CUDA.jl#3219, discovered in JuliaGPU/GPUArrays.jl#754.
`mapreducedim!` reshapes the output container to match the number of dimensions of the input, but then returned that reshaped array instead of the one it was handed. Keep a reference to the original and return that. Same as JuliaGPU/CUDA.jl#3219, discovered in JuliaGPU/GPUArrays.jl#754.
Previously, all the GPU backends returned a reshaped array here, which is inconsistent with base and could cause issues with wrapper types.
|
Can you fix this up into a JLArrays commit and a tests commit and then we won't squash |
|
Ok, I can do that! Looks like Metal tests are still failing though, do you have any ideas why? |
It's unrelated to this. You can work around it by using Good catch! JuliaGPU/Metal.jl#907 |
I don't think it makes sense to promote to `Int` if the result is stored into e.g. an `Int16` array
|
I have pushed a potential fix, which avoids overpromoting in |
* don't return `ReshapedArray` from `mapreducedim!` Discovered in JuliaGPU/GPUArrays.jl#754. Once this is approved, I will open PRs for all the other backends as well. * change other return as well * fix doctest
2 participants
Previously, e.g.
sum!(transpose(v), M)would error