brandon-rhodes / pyephem

Scientific-grade astronomy routines for Python
Other
781 stars 121 forks source link

Modifying satellite parameters #119

Closed dmopalmer closed 5 years ago

dmopalmer commented 7 years ago

I am working on fitting EarthSatellite orbits to observations. This includes creating a satellite from a TLE, then newsat = sat.copy(), and changing attributes such as newsat._n, newsat.catalog_number.

I am getting wild-pointer type behavior, specifically after changing the catalog number (although this may just be the one that manifests in my test case). This behavior includes newsat.catalog_number = 12345 leading to the original sat.catalog_number changing type from int to Angle, a dictionary entry that contained sat changing its key (was the catalog_number, but gets changed to a different integer) and, shortly thereafter, a segmentation violation crash.

So questions: 1) Is it legitimate to modify the orbital parameters, catalog_number, name, etc. of an EarthSatellite object? 2) I notice in _libastro.c that some parameters are in PyGetSetDef EarthSatellite_getset and others are in PyMemberDef EarthSatellite_members. Does that mean 'you can change these but not those'? 3) When you copy a ephem.Body, it calls Body_copy(), I believe. EarthSatellite is a subclass of Body, but it also includes PyObject pointers for name and catalog_number. Do those get properly initialized with pointers to newed PyObjects containing copies?

Sorry I don't have a simple reliable test case.

brandon-rhodes commented 7 years ago

Good evening! It sounds like those are legitimate issues in the C code you have uncovered — what operating system are you using? To address each point:

  1. By adding code to support modification, I had been hoping to successfully support it, but it sounds like my code does not quite make it safe. It may be at the moment that building a new pair of TLE lines and re-reading them might be the safest approach to trying new parameters. (Or trying Skyfield, my replacement for PyEphem which is written in Python and so doesn't have pointer errors!)
  2. No, my intention was definitely to support modifications to both. The difference was simply between primitive values that Python knows how to let people modify, versus special-cased values where I had to write code to intervene.
  3. I am not sure — that could definitely cause problems. The extensions/_libastro.c file is where all of the object-copy logic lives, so if it's not there, then I expect that satellite.copy() is simply an entirely unanticipated case that (a) I failed to think of being a possibility and that (b) the C language happily lets me omit without any warning.

So let's keep this issue open — I'm not sure if PyEphem will be getting much more attention given its fragility and out-of-date astronomical algorithms, but if I do manage to find interested folks who want to work on it at a sprint sometime, having this issue open will given them a high-priority task to take a look at.

I suppose that, since I released Skyfield 1.0 a few days ago, it might be time to update the PyEphem web site to let people know that a more up-to-date alternative exists — I'll see if I can get that done this week! Thanks for the bug report, and I hope you are able to get your calculations working by inputting an updated TLE if not through any other mechanism.

brandon-rhodes commented 5 years ago

Looking back at my earlier work, the copy routine is obviously broken. I've fixed it, so hopefully we'll see a difference the next time PyEphem is released!