Skip to content

Remove bogus AbstractFloat conversion. #419

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

Closed
wants to merge 1 commit into from
Closed

Remove bogus AbstractFloat conversion. #419

wants to merge 1 commit into from

Conversation

tpapp
Copy link
Contributor

@tpapp tpapp commented Nov 7, 2019

Fixes

julia> using ForwardDiff: Dual

julia> AbstractFloat(Dual(1.0, 2.0)) isa AbstractFloat
false

Fixes
```julia
julia> using ForwardDiff: Dual

julia> AbstractFloat(Dual(1.0, 2.0)) isa AbstractFloat
false
```
@tpapp
Copy link
Contributor Author

tpapp commented Nov 7, 2019

FWIW, I think the way forward for the host of stack overflow issues (JuliaMath/SpecialFunctions.jl#186, #324, etc) is to remove the float(::Dual) method as well.

Dual is not <: AbstractFloat, so that method violates its contract.

@KristofferC
Copy link
Collaborator

FWIW, was added in 2e8d016.

@tpapp
Copy link
Contributor Author

tpapp commented Dec 20, 2019

Friendly bump: I am happy to work on this if anything else is required.

@sethaxen
Copy link
Member

Another friendly bump. What needs to be done to get this PR merged?

@nalimilan
Copy link

FWIW, I think the way forward for the host of stack overflow issues (JuliaMath/SpecialFunctions.jl#186, #324, etc) is to remove the float(::Dual) method as well.

Dual is not <: AbstractFloat, so that method violates its contract.

Note that float(::Complex) also returns a Complex, so the contract of that function isn't to return an AbstractFloat. I can't comment on what's the best choice for Dual, though.

@andreasnoack
Copy link
Member

See JuliaMath/SpecialFunctions.jl#186 (comment). The issue with loggamma was fixed a while ago but it keeps showing up every time SpecialFunctions make a new breaking release because older versions of DiffRules didn't have appropriate compat bounds.

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.

6 participants