Cython 7 dec 15 vallado

[interplanetarychris]

I've made the changes for the updated 2015 Vallado version in PR #35 with Cython modifications introduced in PR #36

It is currently failing one unittest, but this is due to my choice to keep the change in spg4 which returns r,v as arrays instead of tuples. The numbers are correct, it's just a difference in ellipses vs brackets.

For my application (a call to sgp4init followed by 3-10 calls to sgp4) it results in about a 2x speedup on my platform (iMac running Mac OS X 10.4.6, with Python 3.7)

I invested a full day in trying to improve the Numba performance, including full declaration of all the variables in all the functions, and jitclass declaration for all the variables in the Satellite() class - however, I wasn't able to prevent it from falling back to object mode at compile time.

[ckuethe]

Why return an array rather than a tuple? This now means that the cython and python version do not produce exactly the same output, nor does the cython version return data as specified in documentation.

I don't have any strong feelings about the tuples vs arrays here but I do want the back ends to be consistent.

[interplanetarychris]

Why return an array rather than a tuple?

An array lends itself to immediate usability in vector math operations (dot products, cross products, etc) which are more likely to be next steps than Python tuple-oriented operations.

[interplanetarychris]

Updated test.py to handle deprecated assertRaisesRegexp for Python 2.6/2.7

[brandon-rhodes]

I know that we have been carrying on conversations about this PR elsewhere, but to keep my thoughts straight I should also duplicate them here on the PR itself!

1. First, thanks very much for exploring how SGP4 can happen faster. This will be crucial for many users for whom Python is not fast enough for this use case.
2. As stated elsewhere, I suspect that trying to move Vallado's C code into a Cython file is a complication that can, happily, be avoided if we leave the Vallado code in a pure .c or .cpp file and write only the few lines of Cython necessary to invoke Vallado from Python. To accomplish this, we can avoid pulling in all the C++ code that Vallado now ships with SGP4, and we can instead pull in only the propagator C file itself. You are correct when you point out that this will define a few extra C symbols that we could otherwise hide, but I believe this is preferable to forking the C code, and then paying the cost of Cython compilation for code that could be accepted by the raw C compiler instead.
3. I am on the fence about whether this sgp4 package itself should be the forum where a compiled SGP4 lives. If we make compilation mandatory, then sgp4 loses its purpose of providing the algorithm on machines that lack a compiler or that cannot install binary modules. But if, as in this PR, we make compilation optional, then we put ourselves in the position of supporting a library that will look different and be implemented differently on different systems, without its users maybe knowing the difference. If instead the compiled SGP4 lived in a different package, then users would have full visibility into which one they were using.
4. But I'll admit: it would make the story a bit more complicated for library developers who want "the fastest possible SGP4 but falling back to pure Python if necessary." What would they do? They couldn't depend on both a compiled SGP4 package and this sgp4 package, because their install would fail if the compiled dependency didn't install. Maybe including the compiled version here is really the only pretty way forward? Let me keep thinking about that.
5. I noticed a new compiled SGP4 available for Python, but its design is not currently optimal (you can't cache the result of parsing a bunch of TLE's, for example, you have to re-parse them for each propagate call you make — if I'm reading it correctly?), and it also segfaulted the first time I tried it. Here's the issue I opened, for anyone curious: Segmentation fault, and a couple of API questions bwinkel/cysgp4#8

Maybe, contrary to my first instinct, we should indeed add a compiled version here with magic invisible fallback, but then provide a call from Python that would tell the user which version they were using, so they could check?

In the meantime, I'll go look for all other issues and PRs touching this effort, to try to make sure I'm up to date on where they all stand, and to try to assure that I'm telling a consistent story in each one.

[brandon-rhodes]

@interplanetarychris — After reading through this PR, it looks to my eyes like the crucial updates to restore full compatibility with upstream Vallado SGP4 that are part of this pull request all landed as part of 55db21e, so I am going to close this PR for now on the assumption that the Cython code, while instructive while we contemplated approaches, is not the direction the project will be taking right now.

If instead there are a few changes in this PR that you think should still be merged, please just comment so that I can re-read and look for them. Thanks again for pushing me to look at ways to get this library faster!

[interplanetarychris]

Agreed - the direct call to the original CPP methods are better than the Cython approximations (without replicating all the code). There still may be reason to use Cython with the SGP4.CPP implementation, but I can build those off the main branch.