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

[RFC] Make ForwardDiffNumbers subtypes of Real #73

Merged
merged 5 commits into from
Dec 7, 2015
Merged

Conversation

jrevels
Copy link
Member

@jrevels jrevels commented Dec 3, 2015

This PR resolve #66 by defining ForwardDiffNumber <: Real, and does some definition bookkeeping to resolve ambiguity warnings and get the tests to pass.

The most notable change (besides the titular one) is that some methods previously defined between Real and ForwardDiffNumber are now defined between ExternalReal and ForwardDiffNumber. ExternalReal is defined in ForwardDiff as:

julia> @eval typealias ExternalReal Union{$(subtypes(Real)...)}
Union{AbstractFloat,Integer,Irrational{sym},Rational{T<:Integer}}

This means that (in theory) custom T<:Real types should work, but only if they're defined before ForwardDiff is used/imported. It might be best to add "ForwardDiff doesn't support custom T<:Real types" in the docs to avoid this confusion all together. Thoughts? On a related note, it'd be helpful if someone else wanted to give the docs a once-over to make sure nothing else needs to be changed in the wake of this PR.

Additionally, this PR (once again, in theory) allows one to have Complex{T<:ForwardDiffNumber} types, so if anybody wants to play around with those, I'd be interested to see how it turns out. I don't think the API methods (gradient, hessian, etc.) will work with complex functions, but experiments that work directly with the number types should be feasible.

@mlubin
Copy link
Contributor

mlubin commented Dec 3, 2015

I think it's fine to exclude custom real types until someone requests it.

@jrevels
Copy link
Member Author

jrevels commented Dec 3, 2015

cc @Scidom, since this PR should help you with your Distributions.jl refactor (once it gets merged and tagged)

@papamarkou
Copy link
Contributor

That's awesome, thanks a lot for doing all this work @jrevels. Yes, I will sort out the Distributions as promised once this gets merged. Moreover, I am nearly there with pushing the long-awaited changes to Lora at some point this month, so I will make sure it can work both with ForwardDiff and ReverseSourceDiff.

jrevels added a commit that referenced this pull request Dec 7, 2015
[RFC] Make ForwardDiffNumbers subtypes of Real
@jrevels jrevels merged commit 32ae0d8 into master Dec 7, 2015
@papamarkou
Copy link
Contributor

Shall we tag a new version and push to metadata? 👍

Before working on Distributions, I will try to integrate ForwardDiff with the new version of Lora, which should take me at most a week, as I am really curious how this would work :)

@jrevels jrevels deleted the realsubtype branch June 15, 2016 22:58
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.

Define ForwardDiffNumber <: Real
3 participants