JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.41k stars 5.45k forks source link

Julia v1.7: Multiplication broken with `BigInt` ranges #41517

Closed dlfivefifty closed 3 years ago

dlfivefifty commented 3 years ago

In Julia v1.7-beta:

julia> 2 * (1:big(100)^100)
ERROR: InexactError: Int64(100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000)
Stacktrace:
 [1] Type
   @ ./gmp.jl:363 [inlined]
 [2] convert
   @ ./number.jl:7 [inlined]
 [3] StepRangeLen{BigInt, BigInt, BigInt}(ref::BigInt, step::BigInt, len::BigInt, offset::Int64)
   @ Base ./range.jl:441
 [4] StepRangeLen (repeats 2 times)
   @ ./range.jl:445 [inlined]
 [5] broadcasted(#unused#::Base.Broadcast.DefaultArrayStyle{1}, #unused#::typeof(*), x::Int64, r::UnitRange{BigInt})
   @ Base.Broadcast ./broadcast.jl:1157
 [6] broadcasted
   @ ./broadcast.jl:1341 [inlined]
 [7] broadcast_preserving_zero_d
   @ ./broadcast.jl:892 [inlined]
 [8] *(A::Int64, B::UnitRange{BigInt})
   @ Base ./arraymath.jl:52
 [9] top-level scope
   @ REPL[9]:1

Previously:

julia> 2 * (1:big(100)^100)
2:2:200000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

The issue is that StepRangeLen only allows Int lengths.

dlfivefifty commented 3 years ago

The PR that changed this is https://github.com/JuliaLang/julia/pull/40320

One quick solution would be to only use StepRangeLen if length is an integer:

_steprangelength(a, st, n::Int) = StepRangeLen(a, st, n)
_steprangelength(a, st, n) = range(a; step=st, length=n)
broadcasted(::DefaultArrayStyle{1}, ::typeof(*), x::Number, r::AbstractRange) = _steprangelength(x*first(r), x*step(r), length(r))

@mcabbott

mcabbott commented 3 years ago

That's a pity, we wondered if there would be edge cases. Re "only use StepRangeLen if length is an Int" [surely] this would mean they can't have zero step. Maybe nobody would ever want both at once?

Do many other things assume length(::AbstractVector)::Int? Arrays too long to fit in 64 bits are pretty big. On Julia 1.6:

julia> ans'
1×100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 adjoint(::StepRange{BigInt, BigInt}) with eltype BigInt:
Error showing value of type Adjoint{BigInt, StepRange{BigInt, BigInt}}:
ERROR: InexactError: Int64(100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000)
Stacktrace:
  [1] Type
    @ ./gmp.jl:362 [inlined]
  [2] convert
    @ ./number.jl:7 [inlined]
  [3] to_index
    @ ./indices.jl:292 [inlined]
dlfivefifty commented 3 years ago

Note the related commit that fixed many of these issues: https://github.com/JuliaLang/julia/pull/37741

I ran into this particular problem in InfiniteArrays.jl, where the length of the array is infinite. This has become an accepted use case:

https://github.com/JuliaLang/julia/blob/3eefaf0a52d1f537c512282a3027ead8e5e4f44e/test/testhelpers/InfiniteArrays.jl

I believe I can work around this particular issue as-is, albeit _steprangelength function would be slightly cleaner. The other solution would be to add a type parameter for the length in StepRangeLen, but that might count as a breaking change.

Though I have to say I don't understand why you didn't change Base.range_start_step_length to return a StepRangeLen?

JeffBezanson commented 3 years ago

From triage: