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

Make AbstractTracer a subtype of Real, not Number #91

Closed
gdalle opened this issue May 22, 2024 · 8 comments · Fixed by #92 or #95
Closed

Make AbstractTracer a subtype of Real, not Number #91

gdalle opened this issue May 22, 2024 · 8 comments · Fixed by #92 or #95
Milestone

Comments

@gdalle
Copy link
Collaborator

gdalle commented May 22, 2024

Increases compatibility with existing code

@gdalle
Copy link
Collaborator Author

gdalle commented May 22, 2024

Would solve control-toolbox/CTDirect.jl#88 (comment)

@jbcaillau
Copy link

@gdalle thank you for feedback. well, if this indeed solves the issue, this is most welcome. given the current type hierarchy, this seems to be no problem provided that no complex numbers are involved 👀.
IMG_3145

@adrhill
Copy link
Owner

adrhill commented May 22, 2024

Sounds reasonable. We won't touch differentiation of complex numbers anyway, so this would only have been relevant for connectivity_pattern on complex functions, which isn't our priority.

I'll tag a last patch release and add this breaking change.

@adrhill
Copy link
Owner

adrhill commented May 22, 2024

Some weirdness resulting from this: our local tracer type Dual can contain a Complex primal value but is Real.
Maybe the most sane fix is to constrain the primal to Real as well.

@gdalle
Copy link
Collaborator Author

gdalle commented May 23, 2024

I agree, and we'll be forever banning complex autodiff (but so did ForwardDiff)

@gdalle
Copy link
Collaborator Author

gdalle commented May 23, 2024

Based on the way tracers are constructed, if the function signature is f(::Vector{<:Real}), the primal values will never be anything but Real anyway

@gdalle
Copy link
Collaborator Author

gdalle commented May 23, 2024

Reopening until #95 is merged

@jbcaillau
Copy link

jbcaillau commented May 23, 2024

I agree, and we'll be forever banning complex autodiff (but so did ForwardDiff)

Indeed, replacing $i^2=-1$ by $\varepsilon^2=0$ is all about ForwardDiff.jl 🤭

@adrhill adrhill added this to the Paper milestone May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants