Skip to content
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

Fix ndims for Broadcasted with no args. #45477

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

N5N3
Copy link
Member

@N5N3 N5N3 commented May 27, 2022

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
New `_maxndims` is based on `tuple_type_tail`, thus improve the inference result.
@N5N3 N5N3 added the broadcast Applying a function over a collection label May 27, 2022
Comment on lines +256 to +259
function _maxndims(Args::Type{<:Tuple{T,Vararg{Any}}}) where {T}
Argstail = Tuple{ntuple(i -> fieldtype(Args, i + 1), fieldcount(Args) - 1)...}
max(_maxndims(Tuple{T}), _maxndims(Argstail))
end
Copy link
Member

Choose a reason for hiding this comment

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

This seems like generally a somewhat worse model for what Types can be present than the reduce it replaces

Copy link
Member Author

@N5N3 N5N3 May 27, 2022

Choose a reason for hiding this comment

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

This is just a type version of tuple’s fold.
Since it makes nested Broadcasted easier to inference, can I say our compiler “prefers” this style?😂

nsajko added a commit to nsajko/julia that referenced this pull request Jan 8, 2025
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 8, 2025
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
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
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
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
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
…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)
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] broadcast Applying a function over a collection bugfix This change fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants