-
Notifications
You must be signed in to change notification settings - Fork 30
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
Replace this package with ForwardDiff's Dual
?
#45
Comments
I would also argue for the educational benefit of keeping such a simple implementation around. The extra layer of indirection with |
I don't think this is an issue, ForwardDiff's Dual{T}(c::Complex{T}) = Complex(Dual(real(c), one(T)), Dual(imag(c), one(T)))
I've never had trouble explaining it to people, but even so, you can easily define a simple dual number (both mathematically and your own implementation) in two slides - why would you need a whole package that duplicates code and divides development efforts just to teach dual numbers to people? If you need them to use the package, just say "This package uses an implementation of And just keep in mind, since complexity is a discussion topic: it's just as easy to write new derivative definitions for |
Are there any downsides of The @MikaelSlevinsky might also be interested. |
Of course, I'm biased towards As previously mentioned, another obvious upside for |
I think the reason it uses But perhaps the approach of |
+1 for this suggestion. The argument about that this package is easy to explain to a class doesn't hold much water to me. A small |
Is the proposal that |
I remove my objection regarding educational benefits. |
Yes (sorry for the late reply). |
Sorry I'm late to the conversation... I'm guessing one pro for Due to that same Risking sounding like a broken record, dual numbers extend their underlying field. From an abstract view, it seems unreasonable to subtype them as reals. |
Yup - this is what I meant by "forcing the better orientation" in earlier discussions.
AFAICT, the only con is that - besides the fact that it doesn't need to support the whole I can't be sure about breaking changes until we actually make the switch, but there shouldn't be any actual loss in functionality.
This has been discussed at significant length; one of the primary discussions is here, and there are a couple of other discussions scattered around if you want to hunt them down. My view is that Julia's type hierarchy establishes the subtype relations which drive dispatch. Borrowing naming conventions from math, while useful for driving design, does not guarantee (or even necessarily imply) that this type hierarchy will obey external formalisms. For AD purposes, |
@MikaelSlevinsky I think the idea is that only |
Another benefit of the immutable MyExtension{T <: Union{Real, Complex, Dual}} <: Number
fields::T
end That would be endless. |
It's annoying to have to port functionality between these two packages. It would be better if there were only be one
Dual
implementation that development efforts can focus on, if we can get away with it. AFAIK, ForwardDiff'sDual
number is strictly superior to the version implemented in this package - in addition to allowing nested dual (i.e. hyper-dual) numbers andN
partials derivatives at once, it features the same performance as this package'sDual
in the single-element case, and is just as easy to write new derivative definitions for.I propose we pull out ForwardDiff's
Dual
type and place it here instead. Of course, we'd want to ensure that we include any derivative definitions here that are missing from ForwardDiff, and vice versa.The only controversial part of this change is that ForwardDiff's
Dual
is a subtype ofReal
, whereas theDual
here is a subtype ofNumber
. I would require that we keep the proposedDual
a subtype ofReal
, since the goal of a dual number implementation is to programmatically support theReal
interface. I really don't think this will matter in any practical sense, but others may disagree.The text was updated successfully, but these errors were encountered: