power uses Float64 exponents for integers #53967

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.

Sign up for GitHub

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

Jump to bottom

Merged

oscardssmith merged 25 commits into JuliaLang:master from KlausC:krc/pow-float-path

Apr 22, 2024

Contributor

KlausC commented Apr 5, 2024 •

edited

Loading

Improve performance of ^(::Float64, n::Integer) in the case of abs(n) > 2^13.

While pow_body is unreliable for abs(n) > 2^25 this implementation provides errors of a few ULPs, while runtime is capped to that of the Float64 implementation.

Fixes #53881
See also #53886.


          power uses Float64 exponents for integers

81a0813

dkarrasch added the maths label

Contributor Author

KlausC commented Apr 6, 2024

@oscardssmith I would be interested in your opinion!


          use_pbs - better runtime prediction

5c60c57

Member

oscardssmith commented Apr 6, 2024

this looks really good to me. I need to spend a little time benchmarking to make sure all the details look good, but I'm theory, this looks great!


          fixed lead_zeros bug introduced previously

c861bb0

KlausC marked this pull request as draft

April 9, 2024 09:05

KlausC added 4 commits

April 10, 2024 12:28


          fixed corner cases and added tests

4ab7026


          use_pbs threshold

bb7633c


          fixed effects in ^

fed55b7


          fix tests for pow

fd7fc04

oscardssmith reviewed

View reviewed changes

base/math.jl Outdated Show resolved Hide resolved

oscardssmith reviewed

View reviewed changes

base/math.jl Outdated Show resolved Hide resolved

oscardssmith reviewed

View reviewed changes

base/math.jl Show resolved Hide resolved

oscardssmith reviewed

View reviewed changes

base/math.jl Outdated Show resolved Hide resolved

oscardssmith reviewed

View reviewed changes

base/math.jl Outdated Show resolved Hide resolved

oscardssmith reviewed

View reviewed changes

base/math.jl Show resolved Hide resolved

oscardssmith reviewed

View reviewed changes

base/special/exp.jl Show resolved Hide resolved


          Merge branch 'master' into krc/pow-float-path

bfbd7d3

oscardssmith reviewed

View reviewed changes

base/math.jl Outdated Show resolved Hide resolved

KlausC and others added 4 commits

April 12, 2024 23:06


          lest @assume_effects

9f8e198


          Merge branch 'master' into krc/pow-float-path

703c047


          simplified use_power_by_squaring

ffe1c25


          Merge branch 'krc/pow-float-path' of https://github.com/KlausC/julia …

df1091d

…into krc/pow-float-path

Contributor Author

KlausC commented Apr 13, 2024 •

edited

Loading

Results of benchmark tests:
The abscissa is sign(n) * log2(abs(n) for integer exponents n.
The ordinate is in time per x^n for x close to but > 1.0.
The first graph show execution times in ns for ´x^n´ power_by_squaring, the second one for n -> exp(log(x)*n).
Hardware: Intel® Core™ i7-3610QM × 8

The result of the merged version with parameters nmin = -2^12, nmax = 3*2^13:

This comment was marked as off-topic.

Sign in to view


          adjusted nmin, nmax

e9b33a2

KlausC marked this pull request as ready for review

April 13, 2024 11:44

KlausC and others added 2 commits

April 13, 2024 13:53


          use clamp

06d0a7c


          Merge branch 'master' into krc/pow-float-path

7212b2d

Contributor Author

KlausC commented Apr 18, 2024

I have no idea, why test compiler/codegen uses ^ as a guinea-pig and also don't see how I should change the code in order to make this test succeed.

KlausC and others added 2 commits

April 18, 2024 18:32


          try to inline Base.Math.exp_impl

9f6d39d


          Merge branch 'master' into krc/pow-float-path

f6c24e1

oscardssmith reviewed

View reviewed changes

base/math.jl Outdated Show resolved Hide resolved


          noinline pow_body, other compiler test

2d6fdf5

oscardssmith approved these changes

View reviewed changes

KlausC added 2 commits

April 19, 2024 14:08


          Merge branch 'master' into krc/pow-float-path

249c53f


          Merge branch 'master' into krc/pow-float-path

2260a80

oscardssmith reviewed

View reviewed changes

base/math.jl Outdated Show resolved Hide resolved

oscardssmith reviewed

View reviewed changes

test/math.jl Outdated Show resolved Hide resolved

Member

oscardssmith commented Apr 21, 2024 •

edited

Loading

Other than the final 2 comments, I think is ready to merge. Also, thank you so much for fixing this! It's great to have a second person who understands this code.


          removed redundant code, mitigate test case

4c7f788

oscardssmith merged commit fe49d56 into JuliaLang:master

4 of 7 checks passed

KlausC deleted the krc/pow-float-path branch

April 22, 2024 07:49

oscardssmith mentioned this pull request

Float64 raised to an integer's typemin produces incorrect result #57463

Open

oscardssmith added bugfix backport 1.10 backport 1.11 labels

oscardssmith mentioned this pull request

Fix Float32/16 raised to integer typemin #57488

Closed

KristofferC pushed a commit that referenced this pull request


          power uses Float64 exponents for integers (#53967)

71ec30c

Improve performance of `^(::Float64, n::Integer)` in the case of `abs(n)
> 2^13`.

While `pow_body` is unreliable for `abs(n) > 2^25` this implementation
provides errors of a few ULPs, while runtime is capped to that of the
`Float64` implementation.

Fixes  #53881
See also #53886.

(cherry picked from commit fe49d56)

This was referenced Mar 11, 2025

Backports release 1.11 #57714

Merged

Backports for 1.10.10 #57715

Open

KristofferC pushed a commit that referenced this pull request


          power uses Float64 exponents for integers (#53967)

237ad1a

Improve performance of `^(::Float64, n::Integer)` in the case of `abs(n)
> 2^13`.

While `pow_body` is unreliable for `abs(n) > 2^25` this implementation
provides errors of a few ULPs, while runtime is capped to that of the
`Float64` implementation.

Fixes  #53881
See also #53886.

(cherry picked from commit fe49d56)

Member

KristofferC commented Apr 10, 2025

Causes a test error on 1.11, won't backport.

KristofferC removed the backport 1.11 label

KristofferC added a commit that referenced this pull request


          Revert "power uses Float64 exponents for integers (#53967)"

This reverts commit 237ad1a.

KlausC restored the krc/pow-float-path branch

April 10, 2025 13:03

Contributor Author

KlausC commented Apr 10, 2025

Causes a test error on 1.11, won't backport.

@KristofferC is there a chance to look at the failed test output? I could not verify that locally.

KlausC mentioned this pull request

Power function broken in Version 1.11.4 #58077

Open

Member

KristofferC commented Apr 11, 2025

It is only on 32-bit. #58062 might fix it. But it is too late for 1.11.5, it can go into 1.11.6 if it is fixed with the earlier-mentioned PR.

The error was https://buildkite.com/julialang/julia-release-1-dot-11/builds/451#019619be-f75a-400d-b019-9d047c239bd3


Test Failed at /cache/build/tester-amdci5-15/julialang/julia-release-1-dot-11/julia-88568a8784/share/julia/test/math.jl:1600
--
  | Expression: Core.Compiler.is_foldable(effects)
  | Context: effects = (+c,+e,+n,!t,+s,+m,+u,+o,+r)
  | T = Float64

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 1.10 bugfix maths