JuliaSpace / SatelliteToolbox.jl

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

Breaking change in `propagate!` #57

Closed ThatcherC closed 3 years ago

ThatcherC commented 3 years ago

I found a breaking change in how propagate! and propagate_to_epoch! are used in v0.9.0 and wanted to make sure it was brought up!

The old usage was (pull from the docs):

o,r,v = propagate!(orbp, collect(0:3:24)*60*60);

whereas the new usage is:

r, v = propagate!(orbp, collect(0:3:24)*60*60);

Running my old programs with SatelliteToolbox v0.9.0, I get the error

ERROR: LoadError: BoundsError: attempt to access Tuple{StaticArrays.SVector{3, Float64}, StaticArrays.SVector{3, Float64}} at index [3]
Stacktrace:
 [1] indexed_iterate(t::Tuple{StaticArrays.SVector{3, Float64}, StaticArrays.SVector{3, Float64}}, i::Int64, state::Int64)
   @ Base ./tuple.jl:86
 [2] top-level scope
   @ ~/path/to/file.jl:28
in expression starting at /home/thatch/path/to/file.jl:28

where the offending line is the second one below:

tleprop = init_orbit_propagator(Val(:sgp4), tle);
_,rTEME,vTEME = propagate_to_epoch!(tleprop,JD); # this is line 28

So it seems like the issue is that my old code is trying to unpack 3 return values from the propagate_to_epoch! function, but in v0.9.0 only two are returned.

It'll be pretty easy to make this change in my code base, but it would be good to have this mentioned in the release notes, or included in the deprecation warnings if possible. I'd imagine a lot of users make use of the propagate functions, so this API change might cause trouble for other users as well.

ronisbr commented 3 years ago

Hi @ThatcherC

Sorry! I completely missed this bug report. Yes, this was on purpose in v0.9.0.

The propagator API, which is described here https://github.com/JuliaSpace/SatelliteToolbox.jl/blob/master/src/orbit/propagators/PROPAGATORS.md, was changed to return only the state vector (position and velocity) or the osculating elements. I removed the first argument, which was the mean elements, because some propagators cannot obtain this easily. For example, if we keep this, then the numerical propagators I want to implement will need to make an interpolation at each propagation to return the mean elements. To avoid that, the propagation functions must only return now the osculating elements. I also add a function called get_mean_elements that will return the mean elements if you want.