JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.48k stars 5.46k forks source link

x^2 / power_by_squaring broken when type can't be converted. #24151

Open dlfivefifty opened 6 years ago

dlfivefifty commented 6 years ago

The issue is that power_by_squaring now assumes that the input type can be converted to the output type, but that is not necessarily the case. Here's a simple example using an ImaginaryNumber type:

julia> struct ImaginaryNumber
       y::Float64
       end

julia> import Base: *

julia> *(a::ImaginaryNumber, b::ImaginaryNumber) = -a.y*b.y
* (generic function with 511 methods)

julia> ImaginaryNumber(1)^2
ERROR: MethodError: Cannot `convert` an object of type ImaginaryNumber to an object of type Float64
This may have arisen from a call to the constructor Float64(...),
since type constructors fall back to convert methods.
Stacktrace:
 [1] to_power_type(::ImaginaryNumber) at ./intfuncs.jl:162
 [2] power_by_squaring at ./intfuncs.jl:176 [inlined]
 [3] ^ at ./intfuncs.jl:212 [inlined]
 [4] literal_pow(::typeof(^), ::ImaginaryNumber, ::Val{2}) at ./intfuncs.jl:221
fredrikekre commented 6 years ago

It requires your own implementation of power_by_squaring on v0.6 too, to work properly:

julia> ImaginaryNumber(1)^1
ERROR: MethodError: no method matching copy(::ImaginaryNumber)
Closest candidates are:
  copy(::Expr) at expr.jl:35
  copy(::IntSet) at intset.jl:11
  copy(::ObjectIdDict) at associative.jl:465
  ...
Stacktrace:
 [1] power_by_squaring(::ImaginaryNumber, ::Int64) at ./intfuncs.jl:162
 [2] literal_pow(::Base.#^, ::ImaginaryNumber, ::Type{Val{1}}) at ./intfuncs.jl:205

julia> ImaginaryNumber(1)^3
ERROR: MethodError: no method matching *(::ImaginaryNumber, ::Float64)
Closest candidates are:
  *(::Any, ::Any, ::Any, ::Any...) at operators.jl:424
  *(::Bool, ::T<:Number) where T<:Number at bool.jl:101
  *(::Float64, ::Float64) at float.jl:379
  ...
Stacktrace:
 [1] power_by_squaring(::ImaginaryNumber, ::Int64) at ./intfuncs.jl:184
 [2] literal_pow(::Base.#^, ::ImaginaryNumber, ::Type{Val{3}}) at ./intfuncs.jl:205

so nothing has really changed, has it?

dlfivefifty commented 6 years ago

If you implement copy and *, your two examples work in 0.6 but not 0.7

Sent from my iPhone

On 15 Oct 2017, at 13:11, Fredrik Ekre notifications@github.com wrote:

It requires your own implementation of power_by_squaring on v0.6 too, to work properly:

julia> ImaginaryNumber(1)^1 ERROR: MethodError: no method matching copy(::ImaginaryNumber) Closest candidates are: copy(::Expr) at expr.jl:35 copy(::IntSet) at intset.jl:11 copy(::ObjectIdDict) at associative.jl:465 ... Stacktrace: [1] power_by_squaring(::ImaginaryNumber, ::Int64) at ./intfuncs.jl:162 [2] literal_pow(::Base.#^, ::ImaginaryNumber, ::Type{Val{1}}) at ./intfuncs.jl:205

julia> ImaginaryNumber(1)^3 ERROR: MethodError: no method matching (::ImaginaryNumber, ::Float64) Closest candidates are: (::Any, ::Any, ::Any, ::Any...) at operators.jl:424 (::Bool, ::T<:Number) where T<:Number at bool.jl:101 (::Float64, ::Float64) at float.jl:379 ... Stacktrace: [1] power_by_squaring(::ImaginaryNumber, ::Int64) at ./intfuncs.jl:184 [2] literal_pow(::Base.#^, ::ImaginaryNumber, ::Type{Val{3}}) at ./intfuncs.jl:205 so nothing has really changed, has it?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

TotalVerb commented 6 years ago

The algorithm should be changed nevertheless to avoid doing the needless conversion.