AndrewAnnex / SpiceyPy

SpiceyPy: a Pythonic Wrapper for the SPICE Toolkit.
MIT License
389 stars 84 forks source link

REQUEST: implement undocumented ev2lin function #178

Closed parkerjon closed 4 years ago

parkerjon commented 7 years ago

Hi Andrew, This is a routine documented in other implementations but not exposed in cspice: https://naif.jpl.nasa.gov/pub/naif/toolkit_docs/FORTRAN/spicelib/ev2lin.html

My request is naive, so I am unaware of the difficulty of implementation or whether it is even possible...

AndrewAnnex commented 7 years ago

hmm not sure, it looks like that function has been translated to c in cspice but is just not exposed in the cspice api. I could take a look at this in a day or two once I resolve why the travis builds are failing. If I can use it directly than I don't think it would be too hard, but I will need some sort of test case and test data to validate it.

AndrewAnnex commented 7 years ago

I don't think this being properly compiled into the shared lib of spice, as I get a segfault the moment I try to access the function. I will probably close this as a won't fix in a few days. The issue being that this routine is not properly integrated into the shared library which would mean the naif would need to provide this function as an officially supported function in the cspice api. That being said there are probably a few other things that one could try including using cffi but that would possibly require a major rework of spiceypy which I haven't had time to look into. Alternatively I could write the equivalent _c.c wrapper file the naif provides for all the other supported functions and inject that into the build step, but I would have to investigate that further to determine how feasible it is.

parkerjon commented 7 years ago

Understood completely, Andrew. I wonder why the routine isn't properly integrated into the cspice library the same way it is in fortran?

It's not a big deal for me to create a python wrapper around the mkspk command line tool and read from a temp file, since initialization performance isn't a concern.

On Jan 15, 2017, at 3:16 PM, Andrew Annex notifications@github.com wrote:

I don't think this being properly compiled into the shared lib of spice, as I get a segfault the moment I try to access the function. I will probably close this as a won't fix in a few days. The issue being that this routine is not properly integrated into the shared library which would mean the naif would need to provide this function as an officially supported function in the cspice api. That being said there are probably a few other things that one could try including using cffi but that would possibly require a major rework of spiceypy which I haven't had time to look into. Alternatively I could write the equivalent _c.c wrapper file the naif provides for all the other supported functions and inject that into the build step, but I would have to investigate that further to determine how feasible it is.

― You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

AndrewAnnex commented 7 years ago

so I got a bit curious about my last proposed solution, and to my surprise I wrote a c file wrapper relatively quickly that might have worked (I used it without a segfault occuring, a low bar for sure). I could use a test case for the function to validate that I am not getting garbage out of my wrapper function, is that something you can help with @parkerjon? See my other unit tests for a general idea, ideally I just need an ET, geophs array, elems array and the expected output state. I will also contact the naif to check if the reason was a failed f2c translation of the original function.

AndrewAnnex commented 7 years ago

so a thought that occured to me is that the segmentation faults I was seeing earlier may be from not defining the arg types ahead of time, so I might even be able to wrap this function without the _c.c wrapper file I wrote. I will experiment with this more soon

parkerjon commented 7 years ago

Andrew, Thanks for being curious about this. :)

I'll create some test data when I get a chance this week.

AndrewAnnex commented 7 years ago

so I emailed last week and am still waiting for a complete response, just fyi.

parkerjon commented 7 years ago

Strange, sorry Andrew, I thought I replied. Thank you again for looking into this!

I will create the test you suggested as soon as I can. Hopefully I can start on it today and then finish it by Tuesday or Wednesday. I'll pass along any questions if I have them.

Jon

On Mon, Jan 23, 2017 at 1:04 AM, Andrew Annex notifications@github.com wrote:

so I emailed last week and am still waiting for a complete response, just fyi.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/AndrewAnnex/SpiceyPy/issues/178#issuecomment-274407354, or mute the thread https://github.com/notifications/unsubscribe-auth/ADLi8mmrEN902DeTCuRcoESlXJNj8aZWks5rVELrgaJpZM4LeoSo .

parkerjon commented 7 years ago

Hi Andrew, Please take a look at the attached test file. I have taken today's ISS data from https://spaceflight.nasa.gov/realdata/sightings/SSapplications/Post/JavaSSOP/orbit/ISS/SVPOST.html

and generated some test data by creating a SPK using mkspk. Hopefully all the information you need is contained within the attached file. The values I generated with SpiceyPy mostly agree with the numbers that you can see on that page, but not 100% exactly. Unfortunately am not knowledgeable enough about it to explain the discrepancy.

-Jon

On Mon, Jan 23, 2017 at 10:05 AM, jon parker parker.jon@gmail.com wrote:

Strange, sorry Andrew, I thought I replied. Thank you again for looking into this!

I will create the test you suggested as soon as I can. Hopefully I can start on it today and then finish it by Tuesday or Wednesday. I'll pass along any questions if I have them.

Jon

On Mon, Jan 23, 2017 at 1:04 AM, Andrew Annex notifications@github.com wrote:

so I emailed last week and am still waiting for a complete response, just fyi.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/AndrewAnnex/SpiceyPy/issues/178#issuecomment-274407354, or mute the thread https://github.com/notifications/unsubscribe-auth/ADLi8mmrEN902DeTCuRcoESlXJNj8aZWks5rVELrgaJpZM4LeoSo .

parkerjon commented 7 years ago

Attachment didn't come through before, here it is. test_tle2lin.zip

AndrewAnnex commented 7 years ago

So I got a response from the NAIF, and it sounds like you may be able to generate a SPK kernel directly from the TLE using mkspk. This sounds like something you may have done already or may be close to doing which, as I understand it, would bypass the need for ev2lin directly. But I can envision reasons that make exposing ev2lin preferable to using mkspk. I would recommend that you contact Ed Wright via email at the NAIF, you can find his contact info here: https://naif.jpl.nasa.gov/naif/contactinfo.html.

parkerjon commented 7 years ago

Andrew, You are correct, I did end up generating a SPK from the TLE, and actually created the test data that way. As a result I do not have a pressing need for the routine to be translated, but agree that having it as part of the toolset would be useful. Since you expressed interest in developing this anyways I hope the test function is useful for you.

Originally we (our production team at AMNH) were interested in getting information about the trajectory of the AQUA and TERRA spacecraft for somewhat crude map processing purposes and the command line route to converting TLE elements to SPK files works well enough for that case.

I will contact Ed Wright if I have any questions about it. Thank you for passing his information on to me.

-Jon

On Mon, Jan 23, 2017 at 1:06 PM, Andrew Annex notifications@github.com wrote:

So I got a response from the NAIF, and it sounds like you may be able to generate a SPK kernel directly from the TLE using mkspk. This sounds like something you may have done already or may be close to doing which, as I understand it, would bypass the need for ev2lin directly. But I can envision reasons that make exposing ev2lin preferable to using mkspk. I would recommend that you contact Ed Wright via email at the NAIF who I have been in contact with about your situation, you can find his contact info here: https://naif.jpl.nasa.gov/naif/contactinfo.html.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/AndrewAnnex/SpiceyPy/issues/178#issuecomment-274568077, or mute the thread https://github.com/notifications/unsubscribe-auth/ADLi8kPi6EcKUcXMQO99vdlNg7lHKCqJks5rVOwXgaJpZM4LeoSo .

AndrewAnnex commented 7 years ago

I am closing this issue for now as it sounds like you were able to work around the problem. I may post my wrapper functions as a gist if that would be helpful. Alternatively I may revisit this if and when I get around to work on a refactoring of spiceypy that I started experimenting with that uses cffi.

AndrewAnnex commented 4 years ago

got a new inquiry about this issue, reopening

AndrewAnnex commented 4 years ago

I think this was do-able and I was just being silly two years ago... need a good unit test example for this function though

djcinsb commented 4 years ago

Here's some data generated from a C++ call to ev2lin, for 3 spacecraft (Lightsail 2, TDRS 6, Iridium 97) at epoch and at some steps from epoch. I'm using (alas) CSPICE0065, the version currently used by GMAT. The file includes the input TLE along with the ev2lin inputs. Let me know if I missed anything you need. ev2linUnitTestData_11-14-2019.txt

djcinsb commented 4 years ago

The data posted 11/14 used the leap second kernel naif0009.tls. I've updated the data to naif0012.tls; the new test data is attached. ev2linUnitTestData_11-17-2019.txt

cgobat commented 1 year ago

As of November 2021, the NAIF has deprecated the ev2lin routine in favor of evsgp4. IMO, attempting to call spiceypy.ev2lin() should at least raise a DeprecationWarning, if its continued existence/inclusion in SpiceyPy is even warranted at all. The note on the FORTRAN documentation page for EV2LIN is pretty explicit:

Deprecated: This routine has been superseded by the SPICELIB routine EVSGP4. ALL TLE evaluations should use that routine. NAIF supports EV2LIN only for backward compatibility.

The fact that there isn't even an equivalent CSPICE documentation page for this function seems like another pretty compelling argument in favor of dropping support for it.

AndrewAnnex commented 1 year ago

@cgobat took me a while to find this comment, this issue has been closed for a while and a new issue would probably have been better. But in response I agree that deprecation warning would be the way to go, but until it is actually unavailable in the source distribution of cspice I don't agree with removing it. There are other instances where fortran functions are included in spiceypy without cspice wrappers because they are essential to unit tests and unlock important functionality, IIRC, although it's been a while since I thought about them.