brandon-rhodes / python-sgp4

Python version of the SGP4 satellite position library
MIT License
376 stars 88 forks source link

sgp4.wrapper.Satrec objects can't be pickled #80

Closed astrojuanlu closed 3 years ago

astrojuanlu commented 3 years ago

...but sgp4.model.Satrec objects can:

In [1]: from sgp4.api import Satrec

In [2]: Satrec
Out[2]: sgp4.wrapper.Satrec

In [3]: line1, line2 = (
   ...:   "1 00005U 58002B   00179.78495062  .00000023  00000-0  28098-4 0  4753",
   ...:   "2 00005  34.2682 348.7242 1859667 331.7664  19.3264 10.82419157413667",
   ...: )
   ...: sat = Satrec.twoline2rv(line1, line2)
   ...: 

In [4]: sat
Out[4]: <sgp4.wrapper.Satrec at 0x5649605d0430>

In [5]: import pickle

In [6]: with open("/tmp/save.pkl", "wb") as fp:
   ...:     pickle.dump(sat, fp)
   ...: 
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-6-f3c37eb8fd19> in <module>
      1 with open("/tmp/save.pkl", "wb") as fp:
----> 2     pickle.dump(sat, fp)
      3 

TypeError: cannot pickle 'Satrec' object

In [7]: from sgp4.model import Satrec

In [8]: sat = Satrec.twoline2rv(line1, line2)

In [9]: with open("/tmp/save.pkl", "wb") as fp:
   ...:     pickle.dump(sat, fp)
astrojuanlu commented 3 years ago

Neither https://pypi.org/project/dill/ nor https://pypi.org/project/joblib/ work in this case.

brandon-rhodes commented 3 years ago

What is the use case for pickles? I wonder if it could be solved with other serialization mechanisms folks are working on, like OMM. Here are a few complications:

  1. We can't, alas, simply dump a binary of the elsetrec struct to a bytestring, because its byte layout will vary with architecture (big-endian vs little; and the size of char and floats will change on different processors).
  2. The elsetrec struct maintains internal variables from the most recent computation that's been performed. I don't know whether these are ever able to affect calculation outcomes in a way that would make a pickled satellite behave differently from a struct freshly populated from its TLE or OMM, but if they did then pickling and unpickling would produce an object with subtly different behavior.
  3. We of course can't use TLE as the internal implementation of what pickle() stores, because it truncates too many of the floating point digits to correctly re-create the object's behavior, if the satellite's elements came through a higher-precision source (like sgp4init() or OMM).

If your use case really does require pickles instead of a more public mechanism for serialization, we might want to try OMM. We would need extensive round-trip testing of OMM before using it, though — I would almost suggest we need a script that takes a sample satellite set, like SGP4-VER.TLE, and sees if they behave the same before and after pickling, but any set of satellites from TLE is going to be low-precision anyway and isn't probably a precisely-enough specified set of parameters to really measure if full precision is surviving the round-trip?

astrojuanlu commented 3 years ago

Hmmm you're right that OMMs and TLEs are exactly what I'd need in this case! I guess this saves some microseconds of doing Satrec.twoline2rv or sgp4.omm.initialize. Not sure if it's worth the effort though, I'd need to do some benchmarking for that.

brandon-rhodes commented 3 years ago

I guess this saves some microseconds…

It might not save any microseconds, since pickle() would probably just call the OMM output routine, and unpickle would just call the OMM import routine; so the cost would be the normal cost of OMM, plus the small cost of an extra function call. (Unless we found a faster format to pickle to and unpickle from? We would have to invent our own way to, say, save a satellite as a binary string, and then implement it in C, to have pickle and unpickle be faster; and I am fairly sure we don't want this sgp4 module to invent yet another way to represent satellites, that would then have to be maintained forever?)

brandon-rhodes commented 3 years ago

A note for my own records: once #79 lands, I will then need to remember to follow up here on #80 to ask “now that export_omm() is available, does that satisfy your need for a high-precision way to store TLEs?”

astrojuanlu commented 3 years ago

Until you commented here I had not considered the idea of using OMMs (or even TLEs) as the serialization format (silly, I know!) but it's probably the way to go. I found this in https://github.com/satellogic/orbit-predictor/issues/117, but through two very simple __setstate__ and __getstate__ methods I worked around the pickling issue.

...But that was before reading your comment https://github.com/brandon-rhodes/python-sgp4/issues/80#issuecomment-748492738 here, so I'd also like to do some actual benchmarking and see if my solution was good enough or could be improved.

In summary: the lack of pickling support for sgp4.wrapper.Satrec objects is not affecting me (personally) at the moment, and quite probably adding some boilerplate code using TLEs or OMMs is good enough for serialization.

brandon-rhodes commented 3 years ago

In summary: the lack of pickling support for sgp4.wrapper.Satrec objects is not affecting me (personally) at the moment, and quite probably adding some boilerplate code using TLEs or OMMs is good enough for serialization.

That's my guess as well! For the moment, then, I am going to close this, as I don't ever expect that we'll add full pickling support for the Satrec C-language objects because of the issues mentioned above. But if you ever find a satellite that won't correctly round-trip through OMM export and then OMM import, then simply open a new issue describing the problem, and we'll get it fixed. Thanks!