Add `Base.checked_pow(x,y)` to `Base.Checked` library #52849

NHDaly · 2024-01-10T17:44:39Z

Performs ^(x, y) but throws OverflowError on overflow.

Example:

julia> 2^62
4611686018427387904

julia> 2^63
-9223372036854775808

julia> checked_pow(2, 63)
ERROR: OverflowError: 2147483648 * 4294967296 overflowed for type Int64

StefanKarpinski · 2024-01-10T18:14:38Z

base/intfuncs.jl

@@ -309,14 +309,16 @@ to_power_type(x) = convert(Base._return_type(*, Tuple{typeof(x), typeof(x)}), x)
    "\nMake x a float matrix by adding a zero decimal ",
    "(e.g., [2.0 1.0;1.0 0.0]^", p, " instead of [2 1;1 0]^", p, ")",
    "or write float(x)^", p, " or Rational.(x)^", p, ".")))
-@assume_effects :terminates_locally function power_by_squaring(x_, p::Integer)
+power_by_squaring(x_, p::Integer) = power_by_squaring_with_op(*, x_, p)


How about just changing this to power_by_squaring(x_, p::Integer; *=*) and then the rest of the function stays the same?

Clever! Sure that sounds fine to me. Are there any perf issues with kwargs these days for a tight mathematical op like ^?

No, keyword args have been fast for a very long time. You could also make it an optional third arg, but this seems like a good place for a keyword in case we want to be able to override specific other operations in the future (although I can't imagine what).

don't you have to do something to make it specialize on the mul operation?

I think as long as you call the function, we do specialize on function args, oscar

It's totally allowed as a variable name:

julia> * = "Hello" "Hello"

... and it works if you use spaces:

julia> f(x; * = *) = x*x f (generic function with 1 method) julia> f(3) 9

The parsing issue is that *= is an operator, which is why you need a space.

The parser error even indicates this:

julia> f(x; *=*) = x*x ERROR: ParseError: # Error @ REPL[3]:1:6 f(x; *=*) = x*x # └┘ ── invalid identifier

If you look closely, the highlighted part is *= not just *.

(*) = (*) is also an interesting face

hah, i missed that -- i just assumed i was misreading the parser's highlighting. I haven't spent much time with the new parser yet and i'm not used to it - still working mostly in 1.9. Thanks all

base/checked.jl

inkydragon

Don't forget to add the new function to the doc.

julia/doc/src/base/math.md

Lines 153 to 154 in cf44b53

    
           Base.Checked.checked_cld 
        
           Base.Checked.add_with_overflow

inkydragon · 2024-01-31T02:59:17Z

Test compiler/ssair failed on test aarch64-apple-darwin

Looks rarely seen, not sure if it's related.

test log

The global RNG seed was 0xaa6cce87493099399c5d4c23b781e1.
Error in testset compiler/ssair:
Test Failed at /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-XG3Q6T6R70.0/build/default-honeycrisp-XG3Q6T6R70-0/julialang/julia-master/julia-ad6f0489c3/share/julia/test/compiler/ssair.jl:552
  Expression: length(ir.stmts) == 2
   Evaluated: 3 == 2
Error in testset compiler/ssair:
Test Failed at /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-XG3Q6T6R70.0/build/default-honeycrisp-XG3Q6T6R70-0/julialang/julia-master/julia-ad6f0489c3/share/julia/test/compiler/ssair.jl:553
  Expression: Meta.isexpr((ir.stmts[1])[:stmt], :invoke)
Error in testset compiler/ssair:
Test Failed at /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-XG3Q6T6R70.0/build/default-honeycrisp-XG3Q6T6R70-0/julialang/julia-master/julia-ad6f0489c3/share/julia/test/compiler/ssair.jl:559
  Expression: length(ir.stmts) == 4
   Evaluated: 5 == 4
Error in testset compiler/ssair:
Test Failed at /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-XG3Q6T6R70.0/build/default-honeycrisp-XG3Q6T6R70-0/julialang/julia-master/julia-ad6f0489c3/share/julia/test/compiler/ssair.jl:560
  Expression: Meta.isexpr((ir.stmts[1])[:stmt], :invoke)
Error in testset compiler/ssair:
Test Failed at /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-XG3Q6T6R70.0/build/default-honeycrisp-XG3Q6T6R70-0/julialang/julia-master/julia-ad6f0489c3/share/julia/test/compiler/ssair.jl:632
  Expression: new_call_idx !== nothing
   Evaluated: nothing !== nothing
Error in testset compiler/ssair:
Test Failed at /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-XG3Q6T6R70.0/build/default-honeycrisp-XG3Q6T6R70-0/julialang/julia-master/julia-ad6f0489c3/share/julia/test/compiler/ssair.jl:633
  Expression: new_call_idx == new_invoke_idx + 1
   Evaluated: nothing == 4

vtjnash · 2024-01-31T03:17:40Z

That seems definitely related, since it is a test for inferability of ^ that this changes. Maybe @aviatesk can take a quick look and see if something is going wrong there, or if the test is wrong?

aviatesk · 2024-01-31T06:34:23Z

The modifications in this PR seem to be reasonable. I resolved test failures by improving the robustness of compiler/ssair.

inkydragon · 2024-01-31T14:14:20Z

REPL test failure not related, fixed by #53134. pr need rebase.

Fixes #52262

Fixes JuliaLang#52262. Performs `^(x, y)` but throws OverflowError on overflow. Example: ```julia julia> 2^62 4611686018427387904 julia> 2^63 -9223372036854775808 julia> checked_pow(2, 63) ERROR: OverflowError: 2147483648 * 4294967296 overflowed for type Int64 ``` Co-authored-by: Jameson Nash <[email protected]> Co-authored-by: Shuhei Kadowaki <[email protected]>

… (#134) Fixes JuliaLang#52262. Performs `^(x, y)` but throws OverflowError on overflow. Example: ```julia julia> 2^62 4611686018427387904 julia> 2^63 -9223372036854775808 julia> checked_pow(2, 63) ERROR: OverflowError: 2147483648 * 4294967296 overflowed for type Int64 ``` Co-authored-by: Nathan Daly <[email protected]> Co-authored-by: Jameson Nash <[email protected]> Co-authored-by: Shuhei Kadowaki <[email protected]> Co-authored-by: Tomáš Drvoštěp <[email protected]>

NHDaly force-pushed the nhd-checked_pow branch 2 times, most recently from 678d00a to a945b7d Compare January 10, 2024 18:06

NHDaly requested a review from LilithHafner January 10, 2024 18:10

NHDaly force-pushed the nhd-checked_pow branch 2 times, most recently from 5e9dc9a to d23da92 Compare January 10, 2024 18:12

StefanKarpinski reviewed Jan 10, 2024

View reviewed changes

NHDaly requested a review from StefanKarpinski January 10, 2024 18:27

NHDaly force-pushed the nhd-checked_pow branch from ee8b2ff to fbf897b Compare January 10, 2024 23:22

NHDaly requested a review from oscardssmith January 10, 2024 23:23

oscardssmith approved these changes Jan 11, 2024

View reviewed changes

inkydragon reviewed Jan 11, 2024

View reviewed changes

vtjnash force-pushed the nhd-checked_pow branch from efa7e96 to ad6f048 Compare January 30, 2024 22:33

vtjnash added maths Mathematical functions merge me PR is reviewed. Merge when all tests are passing labels Jan 30, 2024

vtjnash removed the merge me PR is reviewed. Merge when all tests are passing label Jan 31, 2024

aviatesk self-assigned this Jan 31, 2024

aviatesk force-pushed the nhd-checked_pow branch from ad6f048 to 046c3c5 Compare January 31, 2024 06:33

aviatesk removed their assignment Jan 31, 2024

NHDaly and others added 5 commits January 31, 2024 08:14

Add Base.checked_pow(x,y)

a0132f8

Fixes #52262

Switch to * kwarg for simpler implementation

941544a

Rename * to mul

d99424e

add to docs

328a644

improve the robustness of compiler/ssair

c80808d

inkydragon force-pushed the nhd-checked_pow branch from 046c3c5 to c80808d Compare January 31, 2024 14:14

vtjnash merged commit 8304111 into master Jan 31, 2024
3 of 7 checks passed

vtjnash deleted the nhd-checked_pow branch January 31, 2024 16:50

oscardssmith mentioned this pull request Jan 31, 2024

WIP: Introduce Overflow Checking for ^(::Integer, ::Integer) #21600

Closed

d-netto mentioned this pull request Mar 19, 2024

Backport checked pow RelationalAI/julia#134

Merged

3 tasks

NHDaly mentioned this pull request May 15, 2024

Add Base.checked_pow(::FixedDecimal, ::Integer), now that it's introduced in Base. JuliaMath/FixedPointDecimals.jl#92

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `Base.checked_pow(x,y)` to `Base.Checked` library #52849

Add `Base.checked_pow(x,y)` to `Base.Checked` library #52849

NHDaly commented Jan 10, 2024

StefanKarpinski Jan 10, 2024

NHDaly Jan 10, 2024

StefanKarpinski Jan 10, 2024

oscardssmith Jan 10, 2024

NHDaly Jan 10, 2024

StefanKarpinski Jan 11, 2024

StefanKarpinski Jan 11, 2024

StefanKarpinski Jan 11, 2024

oscardssmith Jan 11, 2024

NHDaly Jan 16, 2024

inkydragon left a comment

inkydragon commented Jan 31, 2024

vtjnash commented Jan 31, 2024

aviatesk commented Jan 31, 2024

inkydragon commented Jan 31, 2024

Add Base.checked_pow(x,y) to Base.Checked library #52849

Add Base.checked_pow(x,y) to Base.Checked library #52849

Conversation

NHDaly commented Jan 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

inkydragon left a comment

Choose a reason for hiding this comment

inkydragon commented Jan 31, 2024

vtjnash commented Jan 31, 2024

aviatesk commented Jan 31, 2024

inkydragon commented Jan 31, 2024

Add `Base.checked_pow(x,y)` to `Base.Checked` library #52849

Add `Base.checked_pow(x,y)` to `Base.Checked` library #52849