JuliaSpace / SatelliteToolbox.jl

A toolbox for satellite analysis written in julia language.
MIT License
248 stars 33 forks source link

Quick fix to rv_to_kepler #72

Closed ausogle closed 2 years ago

ausogle commented 2 years ago

With the current implementation, for the elliptical and equatorial case, information is being lost about the argument of periapsis. The value being returned is always less than 180. This is due to the cos_\omega is being used.

Consider the change: src/orbit/representations/rv.jl Lines 226-228

sin_ω = ee[2] / norm(ee) sin_ω = abs(sin_ω) > 1 ? 0 : sin_ω ω = asin(sin_ω)

I hesitate to make this change myself, as I am not sure what the point of line 227 is other than to prevent errors, however, it used to point to 0 or pi radians. Now, either positive/negative case is being mapped 0.

ausogle commented 2 years ago

You can run the following snippet of code to see the issue.

using SatelliteToolbox: kepler_to_rv, rv_to_kepler

a = 6674.790266053491
e = 0.0055622826070485095
i = 0.0
Ω = 0.0
ω = 330.27258118831503
θ = 42.80749332919855

rr, vv = kepler_to_rv(a*1000, e, deg2rad(i), deg2rad(Ω), deg2rad(ω), deg2rad(θ))
kep_oe = rv_to_kepler(rr, vv, 0)

println("Comparing results
a = $a, $(kep_oe.a/1000)
e = $e, $(kep_oe.e)
i = $i, $(rad2deg(kep_oe.i))
Ω = $Ω, $(rad2deg(kep_oe.Ω))
ω = $ω, $(rad2deg(kep_oe.ω))
θ = $θ, $(rad2deg(kep_oe.f))")
ronisbr commented 2 years ago

Hi @ausogle !

Thanks for the issue! The problem was a verification after that code. I was checking the sign of e[3], which is always 0, instead of e[2]. It should be fixed in master. Can you please test?

ausogle commented 2 years ago

I ran the code in a local context, not using SatelliteToolbox directly, and I think everything is good to go.

So I am relatively new to Julia and so forgive me for the newbie question. In a terminal, I have run import Pkg; Pkg.update("SatelliteToolbox") it did something but had no impact. I have found where the package was cloned in /foo/.julia/packages/SatelliteToolbox and nothing has been changed in months. The version number correctly matches your current version number in Project.toml. It seems like your CI pipelines are working properly. I even tried Pkg.rm("SatelliteToolbox") and Pkg.add("SatelliteToolbox").

Do you know what I am doing wrong?

ronisbr commented 2 years ago

Hi @ausogle !

The problem is caused because I have not tagged a new release yet. Hence, by adding a package, you are using the latest released version. I am doing a lot of changes (breaking changes) in SatelliteToolbox and it will take a while to tag v0.10. However, I will try to tag v0.9.4 with only this change so that you can use normally.

If you do not want to wait, then you need to use the development branch. In this case, you can type ]dev SatelliteToolbox and Julia will use master with the latest commit available.

ausogle commented 2 years ago

Don't worry about it. I tested the snippet and it worked as it should. Spend time on more important things, not pushing another version. You aren't breaking me, it's just a little bit inconvenient. I just have to write 1e-3 instead of 0. I'm far from being used in a real sense anyhow.

Great work with SatelliteToolbox so far!

ronisbr commented 2 years ago

Don't worry! Nowadays it is very easy to tag a new version :) I have already done, v0.9.4 should be available in the next 30 min.

Great work with SatelliteToolbox so far!

Thanks! I am glad it is being useful :)

ausogle commented 2 years ago

Verified with new tag. Everything is good to go! Thanks for taking care of this so quickly. It's nice to see a repo thats so active