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

Ambiguous Promotion Rule for <:Irrational,<:ReverseDiff.TrackedReal{V,D,O}} #100

Closed
UserQuestions opened this issue Dec 20, 2017 · 5 comments · Fixed by #257
Closed

Ambiguous Promotion Rule for <:Irrational,<:ReverseDiff.TrackedReal{V,D,O}} #100

UserQuestions opened this issue Dec 20, 2017 · 5 comments · Fixed by #257

Comments

@UserQuestions
Copy link

I was using ReverseDiff in a function that uses normcdf and observed the following error:

MethodError: promote_rule(::Type{Irrational{:invsqrt2}}, ::Type{ReverseDiff.TrackedReal{Float64,Float64,ReverseDiff.TrackedArray{Float64,Float64,1,Array{Float64,1},Array{Float64,1}}}}) is ambiguous. Candidates:
promote_rule(::Type{R}, ::Type{ReverseDiff.TrackedReal{V,D,O}}) where {R<:Real, V, D, O} in ReverseDiff at HOMEDIR/.julia/v0.6/ReverseDiff/src/tracked.jl:257
promote_rule(::Type{#s268} where #s268<:Irrational, ::Type{T}) where T<:Number in Base at irrationals.jl:17
Possible fix, define
promote_rule(::Type{R<:Irrational}, ::Type{ReverseDiff.TrackedReal{V,D,O}})
ForwardOptimize at macros.jl:126 [inlined]

  • at scalars.jl:12 [inlined]
    normcdf(::ReverseDiff.TrackedReal{ReverseDiff.TrackedReal{Float64,Float64,ReverseDiff.TrackedArray{Float64,Float64,1,Array{Float64,1},Array{Float64,1}}},ReverseDiff.TrackedReal{Float64,Float64,ReverseDiff.TrackedArray{Float64,Float64,1,Array{Float64,1},Array{Float64,1}}},Void}) at norm.jl:15

I suspect that the corrective code to avoid ambiguity looks something like:

promote_rule{R<:Irrational, V, D, O}(::Type{R}, ::Type{ReverseDiff.TrackedReal{V,D,O}}) = TrackedReal{promote_type(R,V),D,O}

Am I correct? If so, is this line worth adding to the repository to avoid similar ambiguities with irrationals in the future?

@UserQuestions
Copy link
Author

As a follow-up, it appears that in addition to the above, when creating tape to compute Hessians of functions with distributions (e.g. Distributions.normcdf), the Distributions package doesn't know how to handle the ReverseDiff.TrackedArray type. For example, when trying to apply ReverseDiff to create the [tape of] the Hessian for Distributions.normcdf, Julia is unable to match

normcdf(::ReverseDiff.TrackedArray{ReverseDiff.TrackedReal{Float64,Float64,ReverseDiff.TrackedArray{Float64,Float64,1,Array{Float64,1},Array{Float64,1}}},ReverseDiff.TrackedReal{Float64,Float64,ReverseDiff.TrackedArray{Float64,Float64,1,Array{Float64,1},Array{Float64,1}}},1,Array{ReverseDiff.TrackedReal{Float64,Float64,ReverseDiff.TrackedArray{Float64,Float64,1,Array{Float64,1},Array{Float64,1}}},1},Array{ReverseDiff.TrackedReal{Float64,Float64,ReverseDiff.TrackedArray{Float64,Float64,1,Array{Float64,1},Array{Float64,1}}},1}})

to either of the closest candidates:

normcdf(!Matched::Real, !Matched::Real, !Matched::Number) at HOME/.julia/v0.6/StatsFuns/src/distrs/norm.jl:16
normcdf(!Matched::Number) at HOME/.julia/v0.6/StatsFuns/src/distrs/norm.jl:15

@UserQuestions
Copy link
Author

Bump

@jrevels
Copy link
Member

jrevels commented Mar 5, 2018

For the OP, it might suffice just to add :Irrational to the list here.

For the second issue, that seems like a problem with whatever target code is calling that function, i.e. it looks external to ReverseDiff (I could be wrong, though). Why is a scalar function being called on an array?

Moelf added a commit to Moelf/ReverseDiff.jl that referenced this issue Jun 9, 2022
@Moelf
Copy link

Moelf commented Jun 9, 2022

actually we should be able to just add AbstractIrrational to that list?

@ElOceanografo
Copy link
Contributor

Just ran into this issue with ::Type{IrrationalConstants.Log2π}. Is there any reason not to add AbstractIrrational to REAL_TYPES? Happy to make a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants