rca / PySPICE

Python wrapper around the NAIF CSPICE library
http://naif.jpl.nasa.gov/naif/
BSD 3-Clause "New" or "Revised" License
38 stars 15 forks source link

using f2py to fill in the gaps #15

Closed AndrewAnnex closed 9 years ago

AndrewAnnex commented 10 years ago

Hey all, so since getting my python 3 version working, I have been struggling to figure out a way to get the rest of the functions in the exclude list for the wrapper working. I really want to be able to write out CK kernels (ckw0# functions) using pyspice, I even went so far as to start a rewrite of the entire spice library in python, which is kind of nuts I admit. But earlier today I successfully figured out how to use f2py (http://cens.ioc.ee/projects/f2py2e/) to get a few of those functions working, for example mxmtg and mxvg. All I really needed to do was add a few "intent" lines to both fortran files for the "out" matrix/vector. It should be possible to do this for the entire fortran toolkit, I just haven't written any code to do this for me. Would it be possible to combine the c wrapper with the parts generated by f2py? Also on an aside, I am not too familiar with how c works, but why is this wrapper wrapping the functions that end with "_c.c", which are just wrappers of the f2c generated code files which end is just ".c". Since python is written in C, wouldn't it be better to wrap the f2c generated code? Maybe because we are preserving Spice types (ie SpiceDouble)? Edit: I didn't write what I meant to ask here, I was thinking since the .c files are already pure C code (or mostly so), wouldn't it be easier to wrap those functions instead of wrapping the _c.c ones?

rca commented 10 years ago

Wow, this sounds like great progress, awesome!

To answer your question about wrapping the _c.c files; I honestly can't remember the reasoning behind that. But, from the work you've done with f2py, might it be cleaner to go with f2py exclusively? Is there something the existing wrapped code provides that f2py does not?

AndrewAnnex commented 10 years ago

I updated my first response as I was unclear, but I went ahead and responded to what you asked.

well I am not sure quite yet.. if you compare a _c.c to a .c file, say ckw01.c and ckw01_c.c you will see that for the parameters of the function, ckw01.c accepts normal c types and I guess some fortran specific stuff, instead of say SpiceInt or ConstSpiceDouble, so I am not sure what would happen if you say called a function from the existing PySpice wrapped functions, and then passed them to a python function that was created by f2py. Perhaps it would be fine because you are connecting the two via python types back and forth, since f2py is a part of numpy, arrays and vectors and such may need to be converted to list or back to numpy arrays depending on what the situation is. I suppose one way to test this would be to get a simple but excluded function such as ckw01 working with f2py, by attempting to write out a kernel using the f2py command that was created via the PySpice command `ckopn. I will try this later this week. As for doing it all in f2py this is something I am thinking about doing, but I have no experience at all with fortran compiling, so I could be banging my head against the wall for hours if I left out some parameter that experienced fortran coders would know to include.

rca commented 10 years ago

You just triggered a vague memory about tying to the NAIF-defined C interface in the _c.c files rather than attempting to call the Fortran interface.

Just to throw another idea out there, I had an offline chat with @michaelaye, who was exploring direct use of the SPICE-generated C library via Python's ctypes interface.

Thoughts on that?

michaelaye commented 10 years ago

I didn't make any progress on that, but, maybe naïvely, I was assuming that because every SPICE function should have a hook in that C library, it should be possibly to find a generic way to find these hooks in the library file. And, IIUC, ctypes was made for this.

A comment on the f2py story: If I remember correctly, FORTRAN is the only true source for the SPICE package, but correct me if you think that's wrong. Therefore, I believe, even NAIF's cspice package has been generated somehow, possibly even using something like f2py? Special types like SpiceDouble have been introduced, presumably, for the generally good idea to generate machine-independent types that produce the same results on any platform your SPICE code is running. I never verified this, but am hoping that the very thorough guys at NAIF did that actually. So in general it would be wise to keep the SpiceDatatypes alive, they are indeed useful.

michaelaye commented 10 years ago

If it would be helpful, I could ask for some input on the SPICE mailing list? The NAIF guys provide usually very long detailed input to complex questions. They will just be without guarantees because they won't be able to officially attach themselves to our efforts here. (They are working for years on a Python interface, but gave Java the priority. yuck)

AndrewAnnex commented 10 years ago

So from what I have found out, they did use something life f2py for cspice. They used f2c to generate those files, as detailed here. I did look at Python ctypes, there is also cffi that looks like a option, but I just don't have enough experience with C to figure out that stuff. I also came across the problem that most of these tools require a dynamic library, not the static ones included with spice, and I again don't know C/makefiles well enough to figure it out. Again I will try to see what I can do with f2py later this week.

As for Jspice (java) it looks like they used the java native interface, which looks much more powerful than using PyArg_ParseTuple, in that it gives you the size of the input and makes deallocating very simple, the only solution I have seen involves manually allocating and freeing up the memory inside the wrapper using c but I never could get it working myself.

please do ask them, the only other way to do it is to do a complete re-write of spice within python, which wouldn't be so bad if they had more clear documentation on the binary format of the kernel files. I have spent many hours reading fortran code trying to work out what is going on with the low level kernel reading/writing, but the code is older than I am, and does not translate well to a python OO context.

michaelaye commented 10 years ago

I'm currently unclear though what the question(s) is/are? ;) Could you list precise question that we could pre-discuss here before we forward them to the SPICE list?

I would not recommend a rewrite though, because I would not like to see this project getting too detached from the original SPICE libraries. Personally, I would trust an automatically parsed conversion more than a one-person rewrite. That is not a prejudgement against your abilities but an extrapolation of the effort that went into SPICE by the NAIF group and I just find it highly unlikely to be a faster approach towards a working solution than a refinement of the currently existing parsed approach.

On Wed, Feb 19, 2014 at 7:01 PM, Apollo117 notifications@github.com wrote:

So from what I have found out, they did use something life f2py for cspice. They used f2c to generate those files, as detailed herehttp://naif.jpl.nasa.gov/pub/naif/toolkit_docs/C/req/cspice.html. I did look at Python ctypes, there is also cffihttps://cffi.readthedocs.org/en/release-0.8/that looks like a option, but I just don't have enough experience with C to figure out that stuff. I also came across the problem that most of these tools require a dynamic library, not the static ones included with spice, and I again don't know C/makefiles well enough to figure it out. Again I will try to see what I can do with f2py later this week.

As for Jspice (java) it looks like they used the java native interface, which looks much more powerful than using PyArg_ParseTuple, the only solution I have seen involves allocating and freeing up the memory inside the wrapper using c but I never could get it working myself.

please do ask them, the only other way to do it is to do a complete re-write of spice within python, which wouldn't be so bad if they had more clear documentation on the binary format of the kernel files. I have spent many hours reading fortran code trying to work out what is going on with the low level kernel reading/writing, but the code is older than I am, and does not translate well to a python OO context.

— Reply to this email directly or view it on GitHubhttps://github.com/rca/PySPICE/issues/15#issuecomment-35582049 .

rca commented 10 years ago

I agree here. Porting decades-old code is not a weekend project. This is a great read on the topic of rewrites, btw.

AndrewAnnex commented 10 years ago

Yah, I definitely don't want to rewrite the entirety of spice! I was just postulating. I just got some promising results from using f2py, I used it to make a wrapper for ckopn and ckcls, and it was able to generate a ck kernel for me and it returned a handle of an integer value that were identical to what occurred when doing the same calls using pyspice. Of course this was just a first step, but again extremely promising.

As for questions I didn't really have any yet, besides to ask if they had tried using f2py yet, and if they did what went wrong, because I don't want to spend all of the time to write a parser if there is some overriding flaw in f2py that prevents it from successfully wrapping spice. If they haven't tried f2py, they should get on that because I got this little part working with a very little amount of time and effort.

below is a abbreviated summary of what I did, but I excluded the build step because I included every fortran source file and include file in spice, which is a rather long list.

f2py code to generate signature (inc file is most likely unnecessary at this stage, but I just left it there it didn't appear to hurt): f2py3 -m ckopen -h ckopen.pyf --overwrite-signature ckopn.f ckparam.inc ckcls.f

Generated signature, all intent( ) portions I added, parser code would allow f2py to generate these for me by going through the fortran files and appending appropriate Cf2py flags, see f2py users guide.

!    -*- f90 -*-
! Note: the context of this file is case sensitive.

python module ckopen ! in 
    interface  ! in :ckopen
        subroutine ckopn(name,ifname,ncomch,handle) ! in :ckopen:ckopn.f
            character*(*) intent(in) :: name
            character*(*) intent(in) :: ifname
            integer intent(in) :: ncomch
            integer intent(out) :: handle
        end subroutine ckopn
        subroutine ckcls(handle) ! in :ckopen:ckcls.f
            integer intent(in) :: handle
        end subroutine ckcls
    end interface 
end python module ckopen

! This file was auto-generated with f2py (version:2).
! See http://cens.ioc.ee/projects/f2py2e/
drbitboy commented 10 years ago

My two cents.

When I were a lad ...

I started using SPICE in 1988 or 1989, so I remember when it was FORTRAN only, and I might have actually done an F2C effort sometime around 1996, that was obsolete when NAIF did a better job a few months later.

The reason for the _c.c version was probably to provide clean(er), and C-like, interfaces:

1) C-like: FORTRAN arguments are always pass-by-reference (though we VMS dinosaurs know the exception), and while that might actually make it easier to do the python interface, pass-by-value is good enough for input arguments, plus it's easier and cleaner coding to not have to put the ampersand in front of non-pointers.

2) Portability: there is always a munging of the FORTRAN routine name: SUBROUTINE CKW01 in FORTRAN becomes ckw01_, but more importantly, BLAH_H may become BLAHH OR BLAH_H__ (one extra trailing underscore per internal underscore in the source name), depending on the OS, so hiding all that in the _c.c interfaces makes user code more portable.

I'm not sure those are the main reasons, or if they are event still accurate.

But back to the main topic of f2py and adding in the currently-excluded routines:

So replacing the parsing of SpiceUsr.h (basically doing a SWIG-like action) with intent() statements should work, but still I think it will be quicker to leave everything where it is and code for the few exceptions. I had been focusing on finding a way to always parse SpiceUsr.h with special paths for some cases; perhaps a better choice would be to hand-code the entire interface routine for the exceptions and just insert them. The problem of course is keeping up with new versions, but the likelihood of interfaces changing in the SPICE toolkit is small; that is what is done for IDL, for all routines I think, and NAIF seems satisfied maintaining that.

As a side note, I recently came upon another python interface being done by Mark Showalter; I think he also uses a SWIG-like approach. I haven't looked at it yet.

On Thu, Feb 20, 2014 at 2:18 AM, Apollo117 notifications@github.com wrote:

Yah, I definitely don't want to rewrite the entirety of spice! I was just postulating. I just got some promising results from using f2py, I used it to make a wrapper for ckopn and ckcls, and it was able to generate a ck kernel for me and it returned a handle of an integer value that were identical to what occurred when doing the same calls using pyspice. Of course this was just a first step, but again extremely promising.

As for questions I didn't really have any yet, besides to ask if they had tried using f2py yet, and if they did what went wrong, because I don't want to spend all of the time to write a parser if there is some overriding flaw in f2py that prevents it from successfully wrapping spice. If they haven't tried f2py, they should get on that because I got this little part working with a very little amount of time and effort.

below is a abbreviated summary of what I did, but I excluded the build step because I included every fortran source file and include file in spice, which is a rather long list.

f2py code to generate signature (inc file is most likely unnecessary at this stage, but I just left it there it didn't appear to hurt): f2py3 -m ckopen -h ckopen.pyf --overwrite-signature ckopn.f ckparam.inc ckcls.f

Generated signature, all intent( ) portions I added, parser code would allow f2py to generate these for me by going through the fortran files and appending appropriate Cf2py flags, see f2py users guide.

! -- f90 -- ! Note: the context of this file is case sensitive.

python module ckopen ! in interface ! in :ckopen subroutine ckopn(name,ifname,ncomch,handle) ! in :ckopen:ckopn.f character() intent(in) :: name character() intent(in) :: ifname integer intent(in) :: ncomch integer intent(out) :: handle end subroutine ckopn subroutine ckcls(handle) ! in :ckopen:ckcls.f integer intent(in) :: handle end subroutine ckcls end interface end python module ckopen

! This file was auto-generated with f2py (version:2). ! See http://cens.ioc.ee/projects/f2py2e/

-- Reply to this email directly or view it on GitHubhttps://github.com/rca/PySPICE/issues/15#issuecomment-35594955 .

rca commented 10 years ago

Thanks for the input! Regarding Mark Showalter, it'd be great to collaborate rather than duplicate effort. Any chance of inviting him into the conversation?

@Apollo117 Do you have an f2py module that can be used to perform an integration experiment? Once the module is built, it can be integrated by adding the appropriate import lines in spice/misc.py. Thoughts?

AndrewAnnex commented 10 years ago

So right now I am going through and getting all of the functions used in the example spice program from the spice tutorials page so that I can see a range of funcitons and how f2py handles generating the signatures. It does a pretty good job, but the by default f2py assumes all parameters are intent(in), so for out parameters I have to manually fix the signature or I would need to add flags to the source files.

That being said, since the signature file is likely universal to any platform it is probably safe to generate the signature once, and to update the signature as needed to get things working. The problem with distributing this with pyspice is that I have had to included the entirety the fortran spice library in the build step for f2py (all .inc files, all 'private' zz functions). So we would need to have the user download both the C and Fortran versions of spice unless there is something I am missing here (I thought this was what the static libraries were for, but I have absolutely no knowledge or understanding about how they work, or how to get them to work f2py if that is possible).

What I plan to do for now is to continue learning about f2py to get all of the example program working, so far I have been able to load kernels and get functions like bodn2c and str2et working after about 10 minutes of work so I anticipate completing this tonight. Next I plan to try to get the major functions I want to be able to use from the exclude_list in mkwrapper.py. Functions like ckw0# and such, the various matrix/vector ones just because and maybe the pool ones. I will probably post a gist or something in the next few days showing my most recent updates to a signature file that you all would be able to test out yourselves.

rca commented 10 years ago

Why not fork this project, make a branch, and then open a pull request. That way when it takes shape it's easy to merge. It's easier than maintaining a gist to boot. :)

AndrewAnnex commented 10 years ago

Sorry for not updating in a bit, but I have some bad news. First of all it doesn't look like you can utilize the same kernel pool when running PySpice and my F2py generated code (I will call it Fspice for now) at the same time, so for example if you loaded a kernel in PySpice and ran str2et with Fspice you get an error, and visa versa. So in order to get these two communicate together you would need to maintain two kernel pools.

Secondly I ran into issues when trying to get getfov working. There doesn't appear to be any issues with the parameters being passed in, but rather internally inside the procedure for getfov when reading the instrument kernel. If you download the Fortran library and go to line number 760 that is where it appears to break apart. From what I can tell is happening is that the low level functions like GCPOOL fail to find the shape for the given instrument (In this case the Cassini ISS Nac imager). I of course have loaded the correct kernels, and I even was able to wrap lower level function contained in KEEPER.F to show that I had loaded kernels.

I am a bit mystified about this not working, it also doesn't help that GETFOV is one of excluded functions in PySpice.

Does anyone here know what is going on? The real problem I see with me continuing to work on this stuff is that I am not a fortran programmer, I can insert print statements to see things going on but that is about it. I don't know enough about how spice works to really get what is going on. What I am going to try next is to wrap those individual functions in keeper and then try to see if I can get those to work alone, but I don't think that would help get the getfov routine working.

The only other solution that I keep seeing would be to compile cspice as a dynamic library instead of the static one included in the distribution, this would allow python to load the module directly using the standard library. Does anyone here know how to do that?

drbitboy commented 10 years ago

problem is probably that cspice.a and spice.a are static libraries, with different static memory (e.g. kernel pool, loaded kernels, etc.)

On Wed, Feb 26, 2014 at 4:08 PM, Apollo117 notifications@github.com wrote:

Sorry for not updating in a bit, but I have some bad news. First of all it doesn't look like you can utilize the same kernel pool when running PySpice and my F2py generated code (I will call it Fspice for now) at the same time, so for example if you loaded a kernel in PySpice and ran str2etwith Fspice you get an error, and visa versa. So in order to get these two communicate together you would need to maintain two kernel pools.

Secondly I ran into issues when trying to get getfov working. There doesn't appear to be any issues with the parameters being passed in, but rather internally inside the procedure for getfov when reading the instrument kernel. If you download the Fortran library and go to line number 760 that is where it appears to break apart. From what I can tell is happening is that the low level functions like GCPOOL fail to find the shape for the given instrument (In this case the Cassini ISS Nac imager). I of course have loaded the correct kernels, and I even was able to wrap lower level function contained in KEEPER.F to show that I had loaded kernels.

I am a bit mystified about this not working, it also doesn't help that GETFOV is one of excluded functions in PySpice.

Does anyone here know what is going on? The real problem I see with me continuing to work on this stuff is that I am not a fortran programmer, I can insert print statements to see things going on but that is about it. I don't know enough about how spice works to really get what is going on. What I am going to try next is to wrap those individual functions in keeper and then try to see if I can get those to work alone, but I don't think that would help get the getfov routine working.

The only other solution that I keep seeing would be to compile cspice as a dynamic library instead of the static one included in the distribution, this would allow python to load the module directly using the standard library. Does anyone here know how to do that?

Reply to this email directly or view it on GitHubhttps://github.com/rca/PySPICE/issues/15#issuecomment-36176773 .

AndrewAnnex commented 10 years ago

well that makes sense, but I am not even using the static library when compiling with f2py, I included every fortran and include file in the build step, so I guess I might be just building another static library. It looks like it is possible to include a static library with f2py but I couldn't get it to work.

rca commented 10 years ago

If I may, let's take a step back; what's root issue again? @Apollo117, in your first post you are looking to get the ckw0* functions working. From the comment in the wrapper ckw05_c (and presumably all its friends?) was excluded because, "how to support SpiceCK05Subtype".

I'm way too removed from SPICE at this point to have any intelligent opinion about implementation, but what would it take to add support for SpiceCK05Subtype? Can a Python class with the necessary attributes be added to objects.py and then add a special case to the wrapper generator that is aware of these parameters and properly performs the C <-> Python conversion?

AndrewAnnex commented 10 years ago

So I had another go today but I instead explored using ctypes to do the wrapping (so no more fortran). To do this I had to convert the cspice.a static library to a dynamic library, which I didn't know how to do until I got lucky today with google search. So far I have had some success, I managed to get vsubg_c, mtxm_c, and vadd_c along with the simple constants functions working. That being said however I am not sure if I will end up running into similar issues that my f2py effort ran into. @michaelaye What did you do when you tried using ctypes, or more importantly when did you run into issues? What "roadblocks" occurred? If it is just a question of being careful with how you specify the types for the function parameters and return values this could be a better approach.

michaelaye commented 10 years ago

I only had the idea and didn't try it out yet. You are in 'new waters'! ;)

On Fri, Mar 7, 2014 at 8:16 AM, Apollo117 notifications@github.com wrote:

So I had another go today but I instead explored using ctypes to do the wrapping (so no more fortran). To do this I had to convert the cspice.astatic library to a dynamic library, which I didn't know how to do until I got lucky today with google search. So far I have had some success, I managed to get vsubg_c, mtxm_c, and vadd_c along with the simple constants functions working. That being said however I am not sure if I will end up running into similar issues that my f2py effort ran into. @michaelaye https://github.com/michaelaye What did you do when you tried using ctypes, or more importantly when did you run into issues? What "roadblocks" occurred? If it is just a question of being careful with how you specify the types for the function parameters and return values this could be a better approach.

— Reply to this email directly or view it on GitHubhttps://github.com/rca/PySPICE/issues/15#issuecomment-36954395 .

AndrewAnnex commented 10 years ago

oh dear... Well then, this sounds like a great thing to work on this weekend. I think I will try replicating my experiment from the f2py effort of getting the functions needed for the "spice example idl program" tutorial working.

AndrewAnnex commented 10 years ago

well, I couldn't stop working on this stuff given the progress I made and now I can't quite believe this, but it looks like I have successfully got the example spice program completely working within python with a ctypes wrapper. I will have to explore using ctype pointers more correctly and consistently, as getfov is working via assuming a fixed output boundary array size, which changes in reality. I verified the returned values against the "same" code running in IDL, using the same kernel files. I will post a more detailed explanation in a day or two, but I will do that by repurposing my existing "pure python" repo to begin constructing a more complete wrapper and to move discussion out of this issue. There I will start the basic library, and I will post the steps I took to compile the shared library, eventually those steps will be baked into a setup.py file. But it remains to be seen if this is method is better than the existing pyspice wrapper. Given how quickly I figured out how to write the ones I have already this could come together rather quickly. One important issue at the moment is that when a spice error is thrown, it appears to cause the python interpreter to close after printing the error message, when it should exit in a safer way like in pyspice, like throwing an exception of some kind.

AndrewAnnex commented 10 years ago

So I have set up my repo for the ctypes wrapper and I have some very rough tests and interactions posted. If any of you are interested in helping me out it would be greatly appreciated.

michaelaye commented 10 years ago

My response is a bit slow/late as we have one of the most important conference next week, so after that I will be fully supporting these activities, but for now just one question: Do I need a Python3 env to be able to test this? I saw in your design goals that it should be working for both though? I'm confused that the headline says 'for Python 3' but goals mention also Python 2.x?

michaelaye commented 10 years ago

Also see here: Apollo117/SpiceyPy#1

AndrewAnnex commented 10 years ago

Well, yes and no. The reason for 'no' is that I am using python 3.3, and strings have some significant differences, mainly that in order to make it work I have had to cast strings as binary strings, essentially using b"Hello" vs "Hello". In my SupportTypes file I have a function that converts a string to a char pointer, which uses .encode() and I am not sure if python 2.x has that. Otherwise there should be no reason that would prevent one from running it using python 2.x , as It should be rather trivial to "fix" the code to work in a python 2.x environment by just changing that function in SupportTypes. I haven't looked into this yet as I have been focusing on getting as many of the wrapper functions written as possible.

I am about to push a newer version that hopefully should have a working setup.py, but I have never made a python package with setup-tools so IDK if it is robust. For me it appears to work although to uninstall it I have to manually delete the created folder in the site-packages directory made by setup.py, so just keep that in mind when testing. The issue is that the .so file must be in some location for libspice.py to find, so you can for example just modify it on your system.

Also you wouldn't happen to be going to LPSC would you? I haven't been able to go for a number of years unfortunately... I have been going to DPS instead.

michaelaye commented 10 years ago

I'm indeed going to LPSC. It fits my activities usually a bit more. DPS is still missing on my list, should come up with some work that fits it better. ;)

AndrewAnnex commented 9 years ago

I completely forgot about this issue I posted, I will close it now.