-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
base: master
Are you sure you want to change the base?
Conversation
And some code clean.
New `_maxndims` is based on `tuple_type_tail`, thus improve the inference result.
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?😂
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.
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.
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.
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.
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.
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)
…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)
Follow up #44061.
This PR makes
collect(Base.broadcast(randn))
works correctly, and improve the inference result.Test added.