Skip to content

Conversation

@jishnub
Copy link
Member

@jishnub jishnub commented Nov 18, 2025

This was broken in #59961, as map deals with trailing singleton axes differently from broadcasting:

julia> map(+, ones(1), ones(1,1)) |> size
(1,)

julia> broadcast(+, ones(1), ones(1,1)) |> size
(1, 1)

This PR limits the new method to the case where the ndims match, in which case there are no trailing axes and the two are equivalent. The alternate approach suggested in #59961 (comment) is to reshape the arrays, but this adds overhead that nullifies the performance improvement for small arrays.

@jishnub jishnub requested a review from adienes November 18, 2025 11:44
@test String(take!(io)) == "Any[#= circular reference @-1 =# 3; 2 4;;; 5 7; 6 8]"
end

@testset "size promotion in addition/subtraction" begin
Copy link
Member

Choose a reason for hiding this comment

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

sometimes 1d is a special case; could you add one or two more test cases comparing 2d to 3d? (and maybe similarly some for 0d which is also often special cased)

@adienes
Copy link
Member

adienes commented Nov 18, 2025

seems kind of a shame to have performance depend on the presence of otherwise inconsequential trailing dimensions but I don't have a better proposal

@adienes adienes added arrays [a, r, r, a, y, s] bugfix This change fixes an existing bug labels Nov 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrays [a, r, r, a, y, s] bugfix This change fixes an existing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants