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

prevpow errors on some BigInt/BigFloat input combinations #57906

Open
abigail-gentle opened this issue Mar 27, 2025 · 2 comments · May be fixed by #49669
Open

prevpow errors on some BigInt/BigFloat input combinations #57906

abigail-gentle opened this issue Mar 27, 2025 · 2 comments · May be fixed by #49669
Labels
bignums BigInt and BigFloat bug Indicates an unexpected problem or unintended behavior maths Mathematical functions

Comments

@abigail-gentle
Copy link

Here is a pretty self explanatory MWE:

julia> prevpow(2, 5.1)
4

julia> prevpow(2, BigFloat(5.1))
ERROR: MethodError: no method matching typemax(::Type{BigInt})
The function `typemax` exists, but no method is defined for this combination of argument types.

Closest candidates are:
  typemax(::Type{LibGit2.Consts.GIT_SUBMODULE_IGNORE})
   @ LibGit2 Enums.jl:217
  typemax(::Type{LibGit2.Consts.GIT_MERGE_FILE_FAVOR})
   @ LibGit2 Enums.jl:217
  typemax(::Type{Int128})
   @ Base int.jl:823
  ...

Stacktrace:
 [1] typemax(x::BigInt)
   @ Base ./float.jl:1039
 [2] prevpow(a::Int64, x::BigFloat)
   @ Base ./intfuncs.jl:570
 [3] top-level scope
   @ REPL[12]:1

julia> prevpow(BigFloat(2), 5.1)
4.0

julia> prevpow(BigInt(2), 5.1)
ERROR: MethodError: no method matching typemax(::Type{BigInt})
The function `typemax` exists, but no method is defined for this combination of argument types.

Closest candidates are:
  typemax(::Type{LibGit2.Consts.GIT_SUBMODULE_IGNORE})
   @ LibGit2 Enums.jl:217
  typemax(::Type{LibGit2.Consts.GIT_MERGE_FILE_FAVOR})
   @ LibGit2 Enums.jl:217
  typemax(::Type{Int128})
   @ Base int.jl:823
  ...

Stacktrace:
 [1] typemax(x::BigInt)
   @ Base ./float.jl:1039
 [2] prevpow(a::BigInt, x::Float64)
   @ Base ./intfuncs.jl:570
 [3] top-level scope
   @ REPL[14]:1

julia> versioninfo()
Julia Version 1.11.4
Commit 8561cc3d68d (2025-03-10 11:36 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: macOS (arm64-apple-darwin24.0.0)
  CPU: 10 × Apple M1 Pro
  WORD_SIZE: 64
  LLVM: libLLVM-16.0.6 (ORCJIT, apple-m1)
Threads: 1 default, 0 interactive, 1 GC (on 8 virtual cores)
Environment:
  JULIA_EDITOR = code
  JULIA_NUM_THREADS = 

The error is coming from a typemax call in the prevpow code.

As far as fixing it goes, adding (x isa BigInt) || ... pushes the problem downstream, where mul_with_overflow isn't defined for a mixture of Int and BigInt, this approach would require changing that line to mul_with_overflow(promote(a,p)...) I do not know if this fits the style, or if it is performant.

First and foremost I think there needs to be more test cases to catch things like this, here is the summary of combinations I have right now, with coverage of the special case a == 2

@testset "prevpow input types" begin
    @test prevpow(3,7) == 3
    @test prevpow(3, BigInt(7)) == 3                broken = true
    @test prevpow(3, BigFloat(7)) == 3              broken = true
    @test prevpow(BigInt(3), 7) ==3                 broken = true
    @test prevpow(BigInt(3), BigInt(7)) == 3        broken = true
    @test prevpow(BigInt(3), BigFloat(7)) == 3      broken = true
    @test prevpow(BigFloat(3), 7) == 3
    @test prevpow(BigFloat(3), BigInt(7)) == 3
    @test prevpow(BigFloat(3), BigFloat(7)) == 3

    @test prevpow(BigInt(2), 7) == 4 
    @test prevpow(BigInt(2), BigInt(7)) == 4
    @test prevpow(BigFloat(2), 7) == 4
    @test prevpow(2, BigInt(7)) == 4
    @test prevpow(2, BigFloat(7)) == 4              broken = true
end
@nsajko nsajko added maths Mathematical functions bignums BigInt and BigFloat labels Mar 27, 2025
@nsajko
Copy link
Contributor

nsajko commented Mar 27, 2025

Regarding BigInt, there's PR #49669, which I forgot about until now.

@abigail-gentle
Copy link
Author

Ah , thank you! I missed this when searching somehow

@nsajko nsajko added the bug Indicates an unexpected problem or unintended behavior label Mar 27, 2025
@nsajko nsajko linked a pull request Mar 27, 2025 that will close this issue
nsajko added a commit to nsajko/julia that referenced this issue Mar 27, 2025
Without this change `prevpow` and `nextpow` fail for, e.g., `BigInt`;
incorrectly throwing a `MethodError`. For example, calls like
`prevpow(3, big"10")` or `nextpow(3, big"10")` fail.

The issue is that the code incorrectly assumes the existence of a
`typemax` method.

Another issue, revealed after fixing the previous one, was a missing
`promote` for the arguments of a `mul_with_overflow` call.

Fixes JuliaLang#57906
nsajko added a commit to nsajko/julia that referenced this issue Mar 30, 2025
Without this change `prevpow` and `nextpow` fail for, e.g., `BigInt`;
incorrectly throwing a `MethodError`. For example, calls like
`prevpow(3, big"10")` or `nextpow(3, big"10")` fail.

The issue is that the code incorrectly assumes the existence of a
`typemax` method.

Another issue, revealed after fixing the previous one, was a missing
`promote` for the arguments of a `mul_with_overflow` call.

Fixes JuliaLang#57906
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 maths Mathematical functions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants