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

Fix typo in literal_pow overload #628

Merged
merged 1 commit into from
Aug 3, 2024

Conversation

jpfairbanks
Copy link
Contributor

When you hit the error condition in literal_pow you get an error with rem2pi in it. That is a copy-paste-edit mistake from a few lines up:

function Base.rem2pi(x::Symbolic, mode::Base.RoundingMode)
    T = symtype(x)
    T <: Number ? term(rem2pi, x, mode) : error_f_symbolic(rem2pi, T)
end

was probably copied into

function Base.literal_pow(::typeof(^), x::Symbolic, ::Val{p}) where {p}
    T = symtype(x)
    T <: Number ? Base.:^(x, p) : error_f_symbolic(rem2pi, T)
end

This PR just fixes the typo.

Copy link
Contributor

github-actions bot commented Aug 3, 2024

Benchmark Results

master 7089c98... master/7089c9820d3f05...
overhead/acrule/a+2 0.727 ± 0.016 μs 0.743 ± 0.021 μs 0.979
overhead/acrule/a+2+b 0.707 ± 0.013 μs 0.716 ± 0.017 μs 0.987
overhead/acrule/a+b 0.258 ± 0.0081 μs 0.254 ± 0.011 μs 1.02
overhead/acrule/noop:Int 26.2 ± 0.93 ns 25 ± 0.06 ns 1.05
overhead/acrule/noop:Sym 0.0367 ± 0.0053 μs 0.0365 ± 0.0051 μs 1.01
overhead/rule/noop:Int 0.0441 ± 0.00051 μs 0.044 ± 0.0011 μs 1
overhead/rule/noop:Sym 0.054 ± 0.0026 μs 0.0546 ± 0.0032 μs 0.99
overhead/rule/noop:Term 0.0543 ± 0.0026 μs 0.0544 ± 0.0028 μs 0.997
overhead/ruleset/noop:Int 0.128 ± 0.0035 μs 0.129 ± 0.0034 μs 0.992
overhead/ruleset/noop:Sym 0.152 ± 0.0041 μs 0.159 ± 0.0043 μs 0.958
overhead/ruleset/noop:Term 3.46 ± 0.12 μs 3.21 ± 0.13 μs 1.08
overhead/simplify/noop:Int 0.14 ± 0.0028 μs 0.147 ± 0.0023 μs 0.952
overhead/simplify/noop:Sym 0.157 ± 0.0035 μs 0.164 ± 0.0038 μs 0.959
overhead/simplify/noop:Term 0.0364 ± 0.0019 ms 0.0362 ± 0.0014 ms 1
overhead/simplify/randterm (+, *):serial 0.0868 ± 0.001 s 0.0855 ± 0.0008 s 1.01
overhead/simplify/randterm (+, *):thread 0.0522 ± 0.032 s 0.0508 ± 0.03 s 1.03
overhead/simplify/randterm (/, *):serial 0.209 ± 0.0052 ms 0.212 ± 0.0059 ms 0.982
overhead/simplify/randterm (/, *):thread 0.242 ± 0.0063 ms 0.241 ± 0.0064 ms 1.01
overhead/substitute/a 0.0631 ± 0.0013 ms 0.0597 ± 0.0014 ms 1.06
overhead/substitute/a,b 0.0553 ± 0.0014 ms 0.0528 ± 0.0015 ms 1.05
overhead/substitute/a,b,c 17.1 ± 0.7 μs 17.7 ± 0.58 μs 0.969
polyform/easy_iszero 30.4 ± 1.7 μs 29 ± 2.1 μs 1.05
polyform/isone 2.79 ± 0.01 ns 2.79 ± 0.01 ns 1
polyform/iszero 1.18 ± 0.027 ms 1.09 ± 0.027 ms 1.08
polyform/simplify_fractions 1.72 ± 0.039 ms 1.56 ± 0.031 ms 1.1
time_to_load 4.49 ± 0.026 s 4.52 ± 0.016 s 0.992

Benchmark Plots

A plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR.
Go to "Actions"->"Benchmark a pull request"->[the most recent run]->"Artifacts" (at the bottom).

Copy link
Member

@bowenszhu bowenszhu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this! This is indeed a typo and your fix looks correct.

@bowenszhu bowenszhu merged commit 681f835 into JuliaSymbolics:master Aug 3, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants