bwinkel / cysgp4

A wrapper around the SGP4 package, for sat TLE calculations
Apache License 2.0
23 stars 2 forks source link

Exception handling #2

Closed cbassa closed 4 years ago

cbassa commented 4 years ago

Using propagate_many on the full space-track catalog is very likely to throw RuntimeErrors due to decayed satellites or unphysical values (e.g. negative eccentricity). These errors are thrown in sgp4lib. Ideally these errors are caught by cysgp4, allowing the offending TLEs to be skipped.

bwinkel commented 4 years ago

Good point. I've had no issues so far, when one uses relatively up-to-date TLEs, but if one has outdated TLEs the chance to get an exception is high indeed. Handling this in the propagate_many function is not straightforward, unfortunately. First of all, the output arrays are created before all calculations (to allow numpy broadcasting). One could try to circumvent this via an additional flag array, which is set to True or False depending on the success of the calculation. The other problem could be speed. Right now, everything happens inside a Cython "nogil" block. Exception handling needs the GIL, which one can temporarily acquire, but only at the cost of the concurrently running OpenMP threads. Perhaps one can handle the C++ exceptions somehow, but I don't know yet.

I'll leave this issue open, until I have come up with a good solution.

cbassa commented 4 years ago

These are up-to-date TLEs, but some are close to decay, which throws these Satellite decayed and Error: (e <= -0.001) errors. I verified this using a modified version of runtest from the sgp4 c++ code. The runtest tool does catch these exceptions.

Would it help to uses nans for position and velocity for those satellites that thew an exception?

bwinkel commented 4 years ago

I think, I figured out a solution. Unfortunately, I couldn't find a way to solve this in the Cython code (dealing with C++ exceptions is not well documented), but by adding a wrapper to the underlying SGP C++ code directly, I created a new function "findPositionNaN", which does the trick.

One can now use the on_error parameter to choose between "raise" and "coerce_to_nan". Default is the former, because not all user may be aware of such problems and it's better to have an error in the first place.

cbassa commented 4 years ago

Many thanks for that solution! It works great.