Cython file for sgp4 functions

[rirze]

This PR includes a Cython version of the propagation.py file. Compiling with Cython improves the runtime of the sgp4 routine by 100%.

In my course of using this library, I have found Numba to be increasing my runtimes for the sgp4 routine, rather than decreasing it. It seems like the @jit is compiling in object mode rather than nopython mode, which has all of the speed improvements. This is because Numba cannot (yet?) efficiently compile a function that has a Python object input. As a result, it compiles in object mode, which allows for Python objects, but does not come with any noticeable speed improvements. In fact, on my machine, Numba compilation increases sgp4 runtime by 2-3x! (Ubuntu 18.04 Windows Subsystem Linux, Python 3.6.4)

In short, I wanted strong performance from the sgp4 routine but did not want to resort to using the C/Fortran code directly. I've worked with Cython before, and it would appear to a good compromise here. I have effectively used the same source code from propagation.py in cpropagation.pyx but with imports from the C math library instead of the standard Python library. This alone cut up my code execution by half.

In this PR, I have edited setup.py to compile the Cython extension if the user has Cython already installed. I've also adjusted model.py to use sgp4 from the Cython extension if it exists.

I hope this explains my use case, which involves making ~1,000,000 calls to sgp4. I could imagine others with similar requirements and I hope that this example illustrates how simple it could be to offer a specific solution.

[brandon-rhodes]

Thanks for thinking of this alternative approach! I should have time next week (I'm traveling at the moment for a conference I just spoke at) to take a look at the code and try it out on my machine. It sounds like Numba would only work if we unleashed it on a sub-function that took only primitive inputs and outputs instead of objects — but, I suppose that even if we made that work well, it would still mean always compiling at runtime instead of having the routine ready at execution time.

It will be slightly tedious to have two copies of the logic that we now need to maintain going forward (for example: there's right now a pull request #35 to make changes to the logic, it didn’t wind up being the final version!), but there's probably no way around that while doing ahead-of-time compilation.

I'll think over the next week on whether I really want the property that the library is different depending on the state of someone's system when they install it — could that trip people up? If so, we'd instead want the cpython version to be a different little library folks install. I'll think about the two possible approaches.

Thanks again!

[ckuethe]

On my ubuntu machine Numba does provide a speedup but I do see the same thing where it can't really do its magic in nopython mode. In #14 (comment) I suggested that very approach of creating a sub-function that takes primitive types rather than objects in order to allow numba to properly optimize.

[ckuethe]

Two thoughts after diffing propagation.py and cpropagation.pyx:

• It looks like you used an older version of propagation.py to create cpropagation.pyx.
• It looks a lot like cpropagation.pyx could be generated from propagation.py ("by steam"). This would prevent having to maintain two copies of the same code.

[rirze]

@ckuethe The Cython file can be optimized a lot more, however. One easy thing that could be done is to add typedefs to each variable in the file. That itself should offer a large performance boost on top of the current runtime.

If @brandon-rhodes were to decide that the speed of the library is a significant enough of a principle and that he was fine with maintaining two copies of the same logic (which many libraries do, take a look at Numpy for example), then the two files would end up being syntactically different from each other. I can't see a stream being a good long term solution here.

[rirze]

@ckuethe Re the diff between propagation.py and cpropagation.pyx. I will update this PR to use the latest version of the logic. How I ended up using this old logic is related to #37

[ckuethe]

Regarding adding typedefs, I don't imagine the function signatures will change much (if at all, because API compatibility) over time; I guess I can try my hand at writing a tool to generate the .pyx with typedefs from the .py... good opportunity for me to learn about cython

[rirze]

Function signatures are one thing sure-- more importantly, inner variables can be typed too in Cython. This is where a lot of the speed optimization comes from. I'll upload a version of the .pyx file with the optimizations that could be done. Maybe that'll give you a better idea of how to build the .pyx code from the .py if possible. (Which sounds very cool, but I have no idea how to do that programmatically!)

[ckuethe]

I just took a whack at adding type information, and I'm now not sure that I see a clean way to automate it. There was a lot of considering the output of cython -a. I'll think about it some more.

On the upside, I got propagation to run literally 10x faster... 1071us to propagate GOES15 with HEAD, down to 107us after cythonizing it.

[ckuethe]

https://github.com/ckuethe/python-sgp4/tree/cython to check out my ugly first draft

[rirze]

That looks pretty good! I think there are some things you could've typed but didn't(?)-- all together that's a great idea of the code will look like in its final form. Also glad you were able to see major speed improvements with your adjustments.

[ckuethe]

Like I say, ugly first draft. I was going with the simple approach of just typing the hot spots.

New version, putting all the declarations up top of each function.

This does give me an idea for a semiautomatic conversion: similar to .pxd files, we could potentially create some sort of annotation file listing what kind of type information should be injected into each function.

[rirze]

@brandon-rhodes Sorry to ping, but did you end up deciding as to the state of this PR? Are you willing to compromise on the 'single source of code' when it comes to a Python version along with a different Cython version of spg4? If so, I'm willing to put in the effort to complete this feature.

[brandon-rhodes]

I have no problem with a ping! I was traveling for the past couple of weeks, and the ping helps draw my attention to issues that people are waiting on when I get back to the keyboard.

As I've been thinking about this PR, I've been wondering: is there a way we can make the Cython module out of two different files, (a) the plain pristine Vallado source code in C + (b) a minimal Cython file that takes an incoming Python call and invokes the Vallado C?

That would put us in the beautiful situation of compiling exactly the spec code, with zero modifications.

Related: I think as a first step I'm going to commit every version of Vallado to the repository in a special directory called Vallado, just so we have a permanent record we can examine and diff.

[rirze]

Hmmm. Ideally, either of those approaches would suffice, if the Vallado code wasn't full of cruft and some unnecessary capabilities (as far as the Python port is concerned).

I've created a repository that has the historical versions of David Vallado's code over the years. Maybe, that'll help in some way?

I don't want to force you to maintain a version of this library that has two different implementations, but I really believe that is the best way to get performance. It may be that this new direction that obtains computation speed in this way may not fit into your vision of this library.

If that is the case, I would simply host a fork/rebuild of SGP4 using only Cython. Of course, I wouldn't prefer this, because this would inevitably cause confusion and drift between the two implementations.

I'd love to hear any thoughts.

[brandon-rhodes]

@rirze Thanks very much for the repository for Vallado’s code, it is very helpful. I had not realized how much the project structure had changed as he moved to more opinionated Microsoft tools; I miss the old days of the simple directory of .cpp files!

Could you expand on your comment about “if the Vallado code wasn't full of cruft and some unnecessary capabilities”? On first glance, it looks to me as though cpp/SGP4/SGP4/SGP4.h and cpp/SGP4/SGP4/SGP4.cpp could be pulled from the Vallado master directly into this project where they would provide what amount to a few basic C functions, though wrapped in a C++ namespace, admittedly. And a very small Cython file could then be written that accepts calls from Python and dispatches them to those pristine Vallado C functions.

Are you interested in trying out something like that yourself? If not — and I’ll understand completely if at this point you don’t have the spare cycles to try a second approach — then I’ll be happy to sit down and try it myself, taking your Cython wrapper and seeing if I can get it to call the pristine SGP4 routines. Just let me know which of us should try!

[rirze]

Sure. Last time I did a line for line translation of functions in SGP4.cpp, there were many lines that could be either removed altogether or reimplemented with better logic. (I don't think Vallado coded these functions in the most efficient way)

Anyways, that was/is my opinion, but in the end, all that matters is performance. At least, as this PR is concerned. I'll create simple Cython wrappers around the pristine Vallado functions and compare those against the ones @ckuethe and I worked on. From there, we can make an evaluation and move forward.

[brandon-rhodes]

Sounds good! If we get pure Vallado wrapped and shipped, then I'll be interested to see how many percent improvement would occur by modifying Vallado, but starting with unmodified code will be a starting baseline that makes me happiest. Thanks!

[brandon-rhodes]

An update — my current thoughts about this PR and the other that stacked on top of it are at:

#38 (comment)

@rirze — Have you made any progress toward wrapping the pure C functions? If so then I don't want to step on your toes and am happy to wait to see the result! If not, I have some free hours this week, and would be happy to make the attempt myself. Let me know if you'd be happy to see me tackle it, otherwise I'll hold off. Thanks again for spurring us to look at this improvement!

[rirze]

From my observations, it seems that wrapping C objects or structs with Cython is very awkward and difficult. I've tried a couple of times to get the wrapper working, but the attempts were hopelessly incapable. For me to continue on this work, I think I would have to simplify the C functions to not use objects as parameters, but that would mean changing the source code again.

I'm not really sure of a way to move forward within these design choices.

I'll gladly step aside for someone else to complete this work since I'm not sure how to proceed. Let me know if there's anything I can do.

[brandon-rhodes]

From my observations, it seems that wrapping C objects or structs with Cython is very awkward and difficult.

Thanks very much for the update, and thanks for the time you took to look at the situation! Armed with that warning, I'll go read Vallado's raw code again myself as I ponder.

And, wow, Vallado's rirze/sgp4-cpp@2f2e014 is a big diff for a "nothing changed" edit to the source code! Thanks so much for maintaining the GitHub mirror of SGP4, which makes it possible for me now to easily track his edits.

[brandon-rhodes]

From my observations, it seems that wrapping C objects or structs with Cython is very awkward and difficult.

Having waded through some Cython examples, I see what you mean — it makes it easy to expose C types to Cython, but not to expose them all the way through to Python without an extra wrapper class. I'll think through whether a traditional Python wrapper with a PyMemberDef might not be preferable to the complexity of Cython, and I'll comment here when I've taken a look.

[brandon-rhodes]

Thanks to you both for inspiring the project to wade back into the choppy waters of binary Python packaging! After seeing several approaches, as well as that of a competing library, I decided to do the simplest thing I could think of and wrapped Vallado's raw code with a lean Python interface:

https://github.com/brandon-rhodes/python-sgp4/blob/d2e2c31d8ea1ad80dc0462c7676994137574b1d9/extension/wrapper.cpp

As I prefer to track Vallado’s raw C++ code, and combine it with simple C wrapper code that can be compiled without any third-party dependencies — only Python and a C compiler — I'm going to go ahead and close this pull request as not quite the solution I was confident I wanted to carry forward through the next several years. But thanks for all the discussion and for illustrating what a Cython solution might look like!

Feel free to make more comments here, or in new issues, if you have questions about the approach. And thanks again for putting Vallado’s history up on GitHub, it was so helpful!