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

ecef_to_geocentric returns error when called with a Vector{Taylor1{Float64}} #6

Closed
PerezHz opened this issue Mar 29, 2024 · 5 comments

Comments

@PerezHz
Copy link

PerezHz commented Mar 29, 2024

Hi!

When trying to run

using SatelliteToolboxTransformations, TaylorSeries
v = Taylor1.(rand(3), 2)
ecef_to_geocentric(v)

the following error is returned:

ERROR: MethodError: no method matching AbstractFloat(::Taylor1{Float64})

Closest candidates are:
  (::Type{T})(::T) where T<:Number
   @ Core boot.jl:792
  AbstractFloat(::Int128)
   @ Base float.jl:269
  AbstractFloat(::Int16)
   @ Base float.jl:266
  ...

Stacktrace:
 [1] float(x::Taylor1{Float64})
   @ Base ./float.jl:294
 [2] float(::Type{Taylor1{Float64}})
   @ Base ./float.jl:311
 [3] ecef_to_geocentric(r_e::Vector{Taylor1{Float64}})
   @ SatelliteToolboxTransformations ~/.julia/packages/SatelliteToolboxTransformations/Gho81/src/reference_frames/geodetic_geocentric.jl:45
 [4] top-level scope
   @ REPL[6]:1

i.e., the error is coming from


where the error occurs since Taylor1{Float64} cannot be converted to a float... maybe a possible fix can be to use promote_type somehow?

@PerezHz
Copy link
Author

PerezHz commented Mar 29, 2024

Btw, our current workaround is to overload this method for Vector{Taylor1{Float64}}, but then we incur in a slight type-piracy 🏴‍☠️, which we would like to avoid 😅. Re: PerezHz/NEOs.jl#62, and in particular this log line

@PerezHz
Copy link
Author

PerezHz commented Mar 29, 2024

That said, beyond the possible type piracy, we would like to be able to use this function without having to re-write it (it's essentially the same code, but without the conversion to float), so we thought it'd be best to ask for help here 😅

@ronisbr
Copy link
Member

ronisbr commented Mar 29, 2024

Hi @PerezHz !

I really need that conversion to float to make the output type consistent (we use these algorithms with Float32 often to mimic what we have in the onboard computer). Hence, we have:

promote_type(Int32, Int64, Float32) |> float
Float32

Otherwise, the algorithm will be promoted to Float64 due to Int64.

I am still not sure what is the best approach. However, since Taylor1 seems to behave like a floating-point number for most of operations, what is the problem to define AbstractFloat(S::Taylor1{T}) where T<:Number = S? It should fix all problems as well.

julia> v = Taylor1.(rand(Float32, 3), 2)
3-element Vector{Taylor1{Float32}}:
  0.22708648 + 𝒪(t³)
   0.6600746 + 𝒪(t³)
   0.6802838 + 𝒪(t³)

julia> Core.AbstractFloat(S::Taylor1{T}) where T<:Number = S

julia> ecef_to_geocentric(v)
( 0.7725128686145345 + 𝒪(t³),  1.2394485 + 𝒪(t³),  0.9747065024345271 + 𝒪(t³))

@PerezHz
Copy link
Author

PerezHz commented Mar 30, 2024

Many thanks for the swift reply @ronisbr! Indeed, this seems in principle like a plausible solution on TaylorSeries side; I'd like to keep this issue open just in case things don't work out as expected if that's okay 😄

@PerezHz
Copy link
Author

PerezHz commented Mar 31, 2024

Fixed by JuliaDiff/TaylorSeries.jl#353, thanks @ronisbr!

@PerezHz PerezHz closed this as completed Mar 31, 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
Development

No branches or pull requests

2 participants