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

Regression: stride errors on 0-dim array #44087

Closed
mkitti opened this issue Feb 9, 2022 · 4 comments · Fixed by #44090
Closed

Regression: stride errors on 0-dim array #44087

mkitti opened this issue Feb 9, 2022 · 4 comments · Fixed by #44090
Assignees
Milestone

Comments

@mkitti
Copy link
Contributor

mkitti commented Feb 9, 2022

Julia 1.7.2:

julia> stride( fill(1), 1)
1

#44027 modified stride.

master-branch

julia> stride( fill(1) , 1)
ERROR: ArgumentError: reducing with add_sum over an empty collection of element type Union{} is not allowed.
You may be able to prevent this error by supplying an `init` value to the reducer.
Stacktrace:
  [1] _empty_reduce_error(f::Any, T::Type)
    @ Base ./reduce.jl:307
  [2] reduce_empty(#unused#::typeof(Base.add_sum), #unused#::Core.TypeofBottom)
    @ Base ./reduce.jl:338
  [3] reduce_empty(op::Base.BottomRF{typeof(Base.add_sum)}, #unused#::Type{Union{}})
    @ Base ./reduce.jl:347
  [4] reduce_empty_iter
    @ ./reduce.jl:371 [inlined]
  [5] reduce_empty_iter
    @ ./reduce.jl:370 [inlined]
  [6] foldl_impl(op::Base.BottomRF{typeof(Base.add_sum)}, nt::Base._InitialValue, itr::Tuple{})
    @ Base ./reduce.jl:49
  [7] mapfoldl_impl(f::typeof(identity), op::typeof(Base.add_sum), nt::Base._InitialValue, itr::Tuple{})
    @ Base ./reduce.jl:44
  [8] mapfoldl(f::Function, op::Function, itr::Tuple{}; init::Base._InitialValue)
    @ Base ./reduce.jl:162
  [9] mapfoldl
    @ ./reduce.jl:162 [inlined]
 [10] #mapreduce#264
    @ ./reduce.jl:294 [inlined]
 [11] mapreduce(f::Function, op::Function, itr::Tuple{})
    @ Base ./reduce.jl:294
 [12] sum(f::Function, a::Tuple{}; kw::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
    @ Base ./reduce.jl:520
 [13] sum(f::Function, a::Tuple{})
    @ Base ./reduce.jl:520
 [14] sum(a::Tuple{}; kw::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
    @ Base ./reduce.jl:549
 [15] sum(a::Tuple{})
    @ Base ./reduce.jl:549
 [16] stride(A::Array{Int64, 0}, k::Int64)
    @ Main ./REPL[7]:4
 [17] top-level scope
    @ REPL[9]:1
@KristofferC KristofferC added this to the 1.8 milestone Feb 9, 2022
@N5N3
Copy link
Member

N5N3 commented Feb 9, 2022

We have a daily PkgEval (https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_date/2022-02/07/report.html).
Some other PKG seems also failed because of this PR.
I'll take a look to decide which value to use.

Looks like Strided.jl prefer length(A).
(Although I think test the exact value of stride(zeros(60,60), 3) is somehow fragile)

@mkitti
Copy link
Contributor Author

mkitti commented Feb 9, 2022

My question is if we can always assume that length exists, but if this was used before then just use that.

@mkitti
Copy link
Contributor Author

mkitti commented Feb 9, 2022

Please add a test while fixing this.

@N5N3
Copy link
Member

N5N3 commented Feb 9, 2022

Looks like length is the first choice before sum(st .* size(A)). But @mbauman is not happy with it. (#35929 (comment))

I'll recover the original behavior before #44027

@N5N3 N5N3 self-assigned this Feb 9, 2022
JeffBezanson pushed a commit that referenced this issue Feb 16, 2022
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this issue Feb 17, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this issue Feb 22, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this issue Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants