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

reshape of BigInt ranges leads to errors due to implicit conversion of dims to Int #40076

Open
jishnub opened this issue Mar 17, 2021 · 0 comments
Labels
bignums BigInt and BigFloat bug Indicates an unexpected problem or unintended behavior ranges Everything AbstractRange

Comments

@jishnub
Copy link
Contributor

jishnub commented Mar 17, 2021

julia> reshape(big(1):big(2)^62, 2, :) # works as expected
2×2305843009213693952 reshape(::UnitRange{BigInt}, 2, 2305843009213693952) with eltype BigInt:
 1  3  5  7   9  11  13  15  17  19  21  23  25  27  29  31  33  35  37  39  41  43  45    4611686018427387899  4611686018427387901  4611686018427387903
 2  4  6  8  10  12  14  16  18  20  22  24  26  28  30  32  34  36  38  40  42  44  46     4611686018427387900  4611686018427387902  4611686018427387904

julia> reshape(big(1):big(2)^63, 2, :) # strange error message because of Int overflow
ERROR: DimensionMismatch("parent has 9223372036854775808 elements, which is incompatible with size (2, 4611686018427387904)")
Stacktrace:
 [1] _throw_dmrs(n::BigInt, str::String, dims::Tuple{Int64, Int64})
   @ Base ./reshapedarray.jl:181
 [2] _reshape(parent::UnitRange{BigInt}, dims::Tuple{Int64, Int64})
   @ Base ./reshapedarray.jl:176
 [3] reshape
   @ ./reshapedarray.jl:112 [inlined]
 [4] reshape(parent::UnitRange{BigInt}, dims::Tuple{Int64, Colon})
   @ Base ./reshapedarray.jl:118
 [5] reshape(::UnitRange{BigInt}, ::Int64, ::Colon)
   @ Base ./reshapedarray.jl:117
 [6] top-level scope
   @ REPL[10]:1

julia> reshape(big(1):big(2)^64, 2, :) # clearer error message, but perhaps this should work? 
ERROR: InexactError: Int64(9223372036854775808)
Stacktrace:
 [1] Type
   @ ./gmp.jl:364 [inlined]
 [2] _reshape_uncolon
   @ ./reshapedarray.jl:129 [inlined]
 [3] reshape(parent::UnitRange{BigInt}, dims::Tuple{Int64, Colon})
   @ Base ./reshapedarray.jl:118
 [4] reshape(::UnitRange{BigInt}, ::Int64, ::Colon)
   @ Base ./reshapedarray.jl:117
 [5] top-level scope
   @ REPL[11]:1
@johnnychen94 johnnychen94 added the bignums BigInt and BigFloat label Mar 17, 2021
@StefanKarpinski StefanKarpinski added the bug Indicates an unexpected problem or unintended behavior label Aug 16, 2021
@nsajko nsajko added the ranges Everything AbstractRange label Aug 16, 2024
jishnub added a commit that referenced this issue Dec 5, 2024
This PR generalizes the `reshape` methods to accept `Integer`s instead
of `Int`s, and adds a `_reshape_uncolon` method for `Integer` arguments.
The current `_reshape_uncolon` method that accepts `Int`s is left
unchanged to ensure that the inferred types are not impacted. I've also
tried to ensure that most `Integer` subtypes in `Base` that may be
safely converted to `Int`s pass through that method.

The call sequence would now go like this:
```julia
reshape(A, ::Tuple{Vararg{Union{Integer, Colon}}}) -> reshape(A, ::Tuple{Vararg{Integer}}) -> reshape(A, ::Tuple{Vararg{Int}}) (fallback)
```
This lets packages define `reshape(A::CustomArray, ::Tuple{Integer,
Vararg{Integer}})` without having to implement `_reshape_uncolon` by
themselves (or having to call internal `Base` functions, as in
JuliaArrays/FillArrays.jl#373). `reshape`
calls involving a `Colon` would convert this to an `Integer` in `Base`,
and then pass the `Integer` sizes to the custom method defined in the
package.

This PR does not resolve issues like
#40076 because this still
converts `Integer`s to `Int`s in the actual reshaping step. However,
`BigInt` sizes that may be converted to `Int`s will work now:
```julia
julia> reshape(1:4, big(2), big(2))
2×2 reshape(::UnitRange{Int64}, 2, 2) with eltype Int64:
 1  3
 2  4

julia> reshape(1:4, big(1), :)
1×4 reshape(::UnitRange{Int64}, 1, 4) with eltype Int64:
 1  2  3  4
```

Note that the reshape method with `Integer` sizes explicitly converts
these to `Int`s to avoid self-recursion (as opposed to calling
`to_shape` to carry out the conversion implicitly). In the future, we
may want to decide what to do with types or values that can't be
converted to an `Int`.

---------

Co-authored-by: Neven Sajko <s@purelymail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bignums BigInt and BigFloat bug Indicates an unexpected problem or unintended behavior ranges Everything AbstractRange
Projects
None yet
Development

No branches or pull requests

4 participants