-
Notifications
You must be signed in to change notification settings - Fork 22
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
Type of sunpos is too restrictive for use with ForwardDiff #73
Comments
I want to help on this issue, but I feel I'm not getting the crux of the problem. We can use multiple dispatch for |
The problem is basically that in AstroLib I used almost everywhere the pattern: f(x::Real) = _f(float(x))
_f(x::AbstractFloat) = ... to ensure type stability, but this breaks using types which aren't The solution is to change the pattern to only f(x::Real) = .... but also make sure that the body of |
PR #76 fixed the signature of
This is because of a limitation of
Honestly, I'm not going to change the return type of the function (changing from a non-allocating tuple to an allocating array) to accomodate limitations in another package, so I'm going to close this issue |
If you really want, now you can manually turn the tuple into an array: julia> ForwardDiff.derivative(x -> [sunpos(x)...], 1.0)
4-element Vector{Float64}:
1.052789959609594
-0.18784711921287695
0.9948661788260018
-1.0107962399285806e-6 |
sunpos
is intended "either a scalar or a vector" as per consistency with the IDL Astronomy User's Library, however the implementation of it does not take advantage of multiple dispatch, rather it relies on a single-dispatch wrapper over that loops over the values of the vector input, does explicit type conversions, and passes it onto a hidden method_sunpos
. This manner of implementation, seemingly based directly on IDL, is not Julian, as it's overly restrictive and gets in the way of metaprogramming and interoperability with other libraries such as ForwardDiff and IntervalRootFinding, useful for differentiating and solving. ForwardDiff tries to generalize over Dual numbers and spits out this type error when it encounters a method defined only over floats:MethodError: no method matching _sunpos(::ForwardDiff.Dual{ForwardDiff.Tag{typeof(f), Float64}, Float64, 1}, ::Bool) Closest candidates are: _sunpos(!Matched::AbstractFloat, ::Bool) at ~/.julia/packages/AstroLib/Yq5lS/src/sunpos.jl:4
A more Julian implementation would define two methods, one defined over reals, and the other defined on vectors of reals, and not force floats.
The text was updated successfully, but these errors were encountered: