Fix irrational raised to a non-literal negative integer power#61298
Fix irrational raised to a non-literal negative integer power#61298lvlte wants to merge 2 commits intoJuliaLang:masterfrom
Conversation
|
I wonder if instead, |
|
I think the most elegant solution would be to define the method function ^(x::Union{AbstractFloat, AbstractIrrational, Rational}, p::Integer)
if p < 0
return power_by_squaring(inv(x), -p)
else
return power_by_squaring(x, p)
end
endIt already exists for |
|
Yes, it would make the result "more consistent" with the literal power implementation : Lines 479 to 489 in 984ad24 And the suggestion by @adienes would also fix the issue for integers (I mean But I also noticed that the result when using the inverse - literal power and power_by_squaring - could be a bit off compared to julia> x, p = pi, -10
(π, -10)
julia> Base.power_by_squaring(inv(x), -p)
1.0678279226861544e-5
julia> x^-10
1.067827922686154e-5
julia> float(x)^p
1.0678279226861537e-5
julia> Float64(big(x)^p)
1.0678279226861534e-5This makes me think |
|
It's very much intentional that it doesn't work for integers; it would be otherwise type unstable. That it does work for literal powers is IMHO a bad decision, makes the language hacky and inconsistent. Maybe there is a good reason it wasn't done for |
|
@araujoms This makes sense. Also maybe the reason |
|
Then it's better to leave the PR as it is; |
|
I don't think just |
|
Argh, of course. And even worse, I have to apologise for all this noise, I was trying to review the PR and take care of a baby simultaneously, and the result is that I performed poorly at both tasks. |
|
odds of this breaking anything seem pretty low but I'd like to run nanosoldier just as a precaution @nanosoldier |
Fixes #61284.
^(x::AbstractIrrational, y::Integer)was hitting (non-explicitly)^(x::Number, p::Integer) = power_by_squaring(x,p).Use
float(x)^pwhich has a specialized method^(x::Float64, n::Integer).