synergetics / stockwell_transform

Stockwell Transform for python3
MIT License
30 stars 12 forks source link

License, and Python 3 #1

Closed jkmacc-LANL closed 5 years ago

jkmacc-LANL commented 6 years ago

Hi. I see that this module segfaults on Python 3. I'm working from a version of the code I found at the NIH site to get it working in Python 3 using ctypes instead of a c-level extension module. Is this your code? I may be able to make a contribution if you're interested, but I think you'd need to add a license.

ixaxaar commented 6 years ago

License added.

I did try to make this work for python3, but the C interface had changed, I did manage to port it to the new interface, which worked in python2.7, but not 3 :(

I suppose there are better FFIs for python and C, and if you are interested you are certainly welcome to make a contribution!

jkmacc-LANL commented 6 years ago

Thanks! I seem to have gotten something running using ctypes, which would side-step the Python and NumPy C API issues (doesn't need stmodule.c, for example). It may be slower, though, as the result array is allocated in NumPy.

Yeah, Cython and CFFI are attractive, but they'd be a heavy dependency for such as small C interface. It really is just a few well-behaved C functions.

jkmacc-LANL commented 6 years ago

I thought I'd read somewhere (can't find it now) that the NIH C code was GPL v2. Do you know the origin of the C code?

ixaxaar commented 6 years ago

I'm not really sure about the NIH code's license. They sure haven't mentioned anything on their website.

As far as the FFI is concerned, the footprint of the FFI should not matter as long as the interfaces are fast. If in case of your ctypes implementation, if allocation happens in numpy, that should not slow things down significantly. ctypes looks good, since a lot of projects depend on it, its interfaces can be considered stable (as in they wont change significantly in the future).

BubbaRich commented 6 years ago

Any progress on Python3 this year? I haven't done Python-C in a few years, but I could dig around it. It was seg faulting in the C code, right?

ixaxaar commented 6 years ago

yep, segfault in C code. I had tried to update to the new FFI here -> 70f467e65f7d3eb06c87ece7109a7135fd86b3ac I don't think I have time for this right now and it might be that @jkmacc-LANL encountered some kind of a roadblock? So yeah some digging around might help!

jkmacc-LANL commented 6 years ago

I got it running in Python 3, but I didn’t use the Python C API. I just plugged into the C Stockwell functions using ctypes. Still awaiting approval for release...

jkmacc-LANL commented 5 years ago

Just got it approved, sorry for the delay. Here's the interface in the module that uses it:

https://github.com/LANL-Seismoacoustics/particleman/blob/master/particleman/st.py https://github.com/LANL-Seismoacoustics/particleman/blob/master/particleman/src/st.c

It's LGLP v3, following the original author's license.
pinging @hyperbolicTom

ixaxaar commented 5 years ago

Wow congrats!

You know, its kinda weird to think that I originally made this for some computational neuroscience analysis thingy but its major users turned out to be Geologists (if I'm not wrong).

BTW, do you plan to merge your changes into this repo or should I put up a pointer to particleman in the readme?

jkmacc-LANL commented 5 years ago

Since I use a modified version of the pyctf st.c inside of particleman directly, I don't think a link is necessary. If the ctypes approach is useful, I think you're welcomed to make those corresponding changes here (subject to LGPL licensing).

BubbaRich commented 5 years ago

Was the main modification to allow it to build with the Python 3 setuptools and link with python 3 code, or were there other changes? I can diff the files if there are other changes to dig through.

I was working on neuroscience signal processing, then audio signal processing, since I discovered the Stockwell Xform. I've got a couple of audio applications I'm interested in when I have time, to characterize some audio and extract some features for some interesting ML algorithms.

thanks!

rich

ke 12. kesäk. 2019 klo 12.19 jkmacc-LANL (notifications@github.com) kirjoitti:

Since I use a modified version of the pyctf https://github.com/hyperbolicTom/pyctf st.c inside of particleman directly, I don't think a link is necessary. If the ctypes approach is useful, I think you're welcomed to make those corresponding changes here (subject to LGPL licensing).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/synergetics/stockwell_transform/issues/1?email_source=notifications&email_token=ABEBKUNVCD5AXPASPZ3KBCDP2EOYVA5CNFSM4EIXBCJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXQ7NTY#issuecomment-501348047, or mute the thread https://github.com/notifications/unsubscribe-auth/ABEBKUL2GDXAXMBLOWILMGDP2EOYVANCNFSM4EIXBCJQ .

-- \ Rich Hammett http://blog.arichneuraltapestry.com / rich.hammett@gmail.com \ Nothing great is achieved without passion, but underneath / the passion there should always be that large impersonal \ survey which sets limits to actions that our passions inspire. / - Bertrand Russell

jkmacc-LANL commented 5 years ago

I just replaced the Python module written in C with one written in Python that uses ctypes to interface with the C Stockwell library. That’s all I did. It avoids the Python C API change from Python 2 to 3 since the ctypes interface is the same. Plus ctypes is way more readable than the Python C API.

hyperbolicTom commented 5 years ago

This works with python3, if anybody is interested

jkmacc-LANL commented 5 years ago

@hyperbolicTom Which works in python3? pyctf?

hyperbolicTom commented 5 years ago

Did the attachment go thru? I sent a .c file

On Fri, Jun 14, 2019, 3:18 PM jkmacc-LANL notifications@github.com wrote:

@hyperbolicTom https://github.com/hyperbolicTom Which works in python3? pyctf?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/synergetics/stockwell_transform/issues/1?email_source=notifications&email_token=ABMKERV6OJD4V6CHQXVGLGDP2PVGVA5CNFSM4EIXBCJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXXF7Q#issuecomment-502231806, or mute the thread https://github.com/notifications/unsubscribe-auth/ABMKERWPGDDFSYOIJODP3XTP2PVGVANCNFSM4EIXBCJQ .

hyperbolicTom commented 5 years ago

Well anyway I updated st.c recently, it'll compile under py2 or py3. It's available as part of pyctf but I can email it assuming attachments work

On Fri, Jun 14, 2019, 3:22 PM Tom Holroyd hyperbolic.tom@gmail.com wrote:

Did the attachment go thru? I sent a .c file

On Fri, Jun 14, 2019, 3:18 PM jkmacc-LANL notifications@github.com wrote:

@hyperbolicTom https://github.com/hyperbolicTom Which works in python3? pyctf?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/synergetics/stockwell_transform/issues/1?email_source=notifications&email_token=ABMKERV6OJD4V6CHQXVGLGDP2PVGVA5CNFSM4EIXBCJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXXF7Q#issuecomment-502231806, or mute the thread https://github.com/notifications/unsubscribe-auth/ABMKERWPGDDFSYOIJODP3XTP2PVGVANCNFSM4EIXBCJQ .

jkmacc-LANL commented 5 years ago

I don't think attachments work, but links do. I think this is the file you mean, for reference. Thanks!