micropython-ulab
micropython-ulab copied to clipboard
Overlap and Add algorithm for fast convolution of long array
Hi everyone,
This an implementation of the oaconvolve method that may be faster than convolve espiecielly when dealing with long arrays.
Few things I'm not sure:
- I don't think this will work without complex support and I'm not sure how to change the code to do so
- I'm not sure there si a need for m_free at the end of the method
- I just realized there are micropython contributing guidelines that I probably did not completely follow
The method takes the same arguments as the convolve method.
I'm a bit confused: we agreed that oaconvolve is part of scipy.signal, so numpy.c, and filter.c shouldn't be modified at all. You should also add tests, documentation, and move the version number in ulab.c.
Pff...sorry i forgot to move the method to signal, left it where i built it in filter.
I also saw it didn't built since i added the m_free that was not well used (IS it useful to free the buffer at the end or it will be free anyway when the method returns?)
I'll modify the PR to address your remarks.
And I think that the problem of complexes has to be solved. It wouldn't make sense to include complex support for the sake of a single method.
The checks fail here: https://github.com/v923z/micropython-ulab/actions/runs/12020316249/job/33509066852?pr=695#step:8:317.
I also saw it didn't built since i added the m_free that was not well used (IS it useful to free the buffer at the end or it will be free anyway when the method returns?)
In this case, it shouldn't really matter.
added:
- Support without complex number
- Documentation for oaconvolve
- Bump ulab version
- Test for oaconvolve (same as convolve but with reduced absolute tolerance, it seems the method is not as accurate as convolve)
added:
* Support without complex number * Documentation for oaconvolve * Bump ulab version * Test for oaconvolve (same as convolve but with reduced absolute tolerance, it seems the method is not as accurate as convolve)
Many thanks! I think it's almost ready to go. I added a couple of small comments. If those can be resolved, this could be merged.