Skip to content

Fix ndims for Broadcasted with no args #45477

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Oct 6, 2025
Merged

Fix ndims for Broadcasted with no args #45477

merged 9 commits into from
Oct 6, 2025

Conversation

N5N3
Copy link
Member

@N5N3 N5N3 commented May 27, 2022 •
edited
Loading

Follow up #44061.
This PR makes collect(Base.broadcast(randn)) works correctly, and improve the inference result.
Test added.

N5N3 added 3 commits May 27, 2022 13:44
@N5N3
code move.
574515b
@N5N3
Fix ndims for Broadcasted with no args.
f2c313d 
And some code clean.
@N5N3
make inference on nested more "tough".
7c0c13a 
New `_maxndims` is based on `tuple_type_tail`, thus improve the inference result.
N5N3
N5N3 commented May 27, 2022
View reviewed changes
@N5N3 N5N3 added the broadcast Applying a function over a collection label May 27, 2022
vtjnash
vtjnash reviewed May 27, 2022
View reviewed changes
@oxinabox oxinabox mentioned this pull request Jun 6, 2022
Closed
nsajko added a commit to nsajko/julia that referenced this pull request Jan 8, 2025
@nsajko
broadcast: align ndims implementation with intent behind code
d09703e 
The `N<:Integer` constraint was nonsensical, given that
`(N === Any) || (N isa Int)`. N5N3 noticed this back in 2022:
JuliaLang#44061 (comment)

Follow up on JuliaLang#44061. Also xref JuliaLang#45477.
@nsajko nsajko mentioned this pull request Jan 8, 2025
Merged
nsajko added a commit to nsajko/julia that referenced this pull request Jan 8, 2025
@nsajko
broadcast: align ndims implementation with intent behind code
f53d3a2 
The `N<:Integer` constraint was nonsensical, given that
`(N === Any) || (N isa Int)`. N5N3 noticed this back in 2022:
JuliaLang#44061 (comment)

Follow up on JuliaLang#44061. Also xref JuliaLang#45477.
@nsajko nsajko added arrays [a, r, r, a, y, s] bugfix This change fixes an existing bug labels Jan 8, 2025
nsajko added a commit to nsajko/julia that referenced this pull request Jan 9, 2025
@nsajko
broadcast: align ndims implementation with intent behind code
4f8e44b 
The `N<:Integer` constraint was nonsensical, given that
`(N === Any) || (N isa Int)`. N5N3 noticed this back in 2022:
JuliaLang#44061 (comment)

Follow up on JuliaLang#44061. Also xref JuliaLang#45477.
nsajko added a commit to nsajko/julia that referenced this pull request Jan 9, 2025
@nsajko
broadcast: align ndims implementation with intent behind code
eb66b6d 
The `N<:Integer` constraint was nonsensical, given that
`(N === Any) || (N isa Int)`. N5N3 noticed this back in 2022:
JuliaLang#44061 (comment)

Follow up on JuliaLang#44061. Also xref JuliaLang#45477.
N5N3 pushed a commit that referenced this pull request Jan 10, 2025
@nsajko
broadcast: align ndims implementation with intent behind code (#56999)
d3964b6 
The `N<:Integer` constraint was nonsensical, given that `(N === Any) ||
(N isa Int)`. N5N3 noticed this back in 2022:
#44061 (comment)

Follow up on #44061. Also xref #45477.
KristofferC pushed a commit that referenced this pull request Jan 13, 2025
@nsajko
broadcast: align ndims implementation with intent behind code (#56999)
03ffe4d 
The `N<:Integer` constraint was nonsensical, given that `(N === Any) ||
(N isa Int)`. N5N3 noticed this back in 2022:
#44061 (comment)

Follow up on #44061. Also xref #45477.

(cherry picked from commit d3964b6)
nsajko added a commit to nsajko/julia that referenced this pull request Feb 9, 2025
@nsajko
broadcast: align ndims implementation with intent behind code (Juli…
d7b0004 
…aLang#56999)

The `N<:Integer` constraint was nonsensical, given that `(N === Any) ||
(N isa Int)`. N5N3 noticed this back in 2022:
JuliaLang#44061 (comment)

Follow up on JuliaLang#44061. Also xref JuliaLang#45477.

(cherry picked from commit d3964b6)
@nsajko nsajko mentioned this pull request Feb 9, 2025
Closed
@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Oct 1, 2025
@DilumAluthge DilumAluthge changed the title Fix ndims for Broadcasted with no args. Fix ndims for Broadcasted with no args Oct 3, 2025
@DilumAluthge
Copy link
Member

CI failures seem related:

Error in testset broadcast:
Error During Test at /cache/build/tester-amdci5-14/julialang/julia-master/julia-6f197a01a9/share/julia/test/broadcast.jl:931
  Test threw exception
  Expression: collect(Iterators.product(bc4, bc4)) == collect(Iterators.product(copy(bc4), copy(bc4)))
  MethodError: no method matching _maxndims(::Type{typeof(*)})
  The function `_maxndims` exists, but no method is defined for this combination of argument types.
  Closest candidates are:
    _maxndims(::Type{Tuple{}})
     @ Base broadcast.jl:272
    _maxndims(::Type{Tuple{T}}) where T
     @ Base broadcast.jl:273
    _maxndims(::Type{<:Tuple{T, Vararg{Any}}}) where T
     @ Base broadcast.jl:274
  Stacktrace:

@DilumAluthge DilumAluthge added failing CI is failing. Needs attention. No need to re-run CI. and removed merge me PR is reviewed. Merge when all tests are passing labels Oct 3, 2025
@adienes adienes removed the failing CI is failing. Needs attention. No need to re-run CI. label Oct 5, 2025
N5N3
N5N3 commented Oct 5, 2025
View reviewed changes
@N5N3 N5N3 added the merge me PR is reviewed. Merge when all tests are passing label Oct 5, 2025
@N5N3 N5N3 merged commit 668047d into JuliaLang:master Oct 6, 2025
6 of 8 checks passed
@N5N3 N5N3 removed the merge me PR is reviewed. Merge when all tests are passing label Oct 6, 2025
@N5N3 N5N3 deleted the broad_ndims branch October 6, 2025 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vtjnash vtjnash vtjnash left review comments

Assignees

No one assigned

Labels

arrays [a, r, r, a, y, s] broadcast Applying a function over a collection bugfix This change fixes an existing bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@N5N3 @DilumAluthge @vtjnash @nsajko @adienes