search

xtensor-stack / xtensor

C++ tensors with broadcasting and lazy computing
BSD 3-Clause "New" or "Revised" License
3.36k stars 398 forks source link

[Feature] Xtensor should support basic signal processing #2590

Closed spectre-ns closed 2 years ago

spectre-ns commented 2 years ago

I can image one of the best use cases of xtensor is signal processing and algorithm development. We currently use numpy and scipy for algorithm development at work which makes the effort to performant C++ difficult depending on what calls from scipy get used. I'm currently implementing a FFT based convolution and I would like to add a time domain convolution call as well to implement basic LTI systems like filters. The calls I would like to add are as follows:

  1. Convolve1D(x,y)
  2. Convolve2D(x,y)
  3. Filter(a,b,x)
  4. FiltFilt(a,b,x)

These would allow the application of FIR and IIR filters on signals. These can be implemented as streaming and standalone calls. Streaming is moderately more complex to preserve the accumulation buffers. Filter design would be fun to implement but realistically this is probably better done in matlab or python even if properly implemented in C++. Would be fun to see in binder though...

Would the maintainers accept a PR, @tdegeus? Is their interest in these features? And is anyone interested in helping?

JohanMabille commented 2 years ago

Hi,

There would be definitely an interest in such tools, and any PR would be super welcome. The only question is where should these functions live? We try to keep xtensor as small as possible, with only the expression system core and basic features. More advanced features are in their dedicated repos, that depend on xtensor. For instance, all the linalg operations live in xtensor-blas.

So we should probably create a new repo for that (xtensor-signal-processing? maybe we can find a better and shorter name). For Convolve and Convolve2D, I don't know where to put them. I know they are part of numpy "core", however they seem a bit more advanced features than what we have in xtensor.

spectre-ns commented 2 years ago

@JohanMabille I agree as making the library too big will increase parsing times for the compiler even if not used directly. I would agree with a separate repo. Can we align the repo names with the scipy modules? So this case would be Scipy.signal.

spectre-ns commented 2 years ago

@JohanMabille I also have an implementation of a Savitzky–Golay filter which is definitely too advanced for base xtensor.

JohanMabille commented 2 years ago

Can we align the repo names with the scipy modules? So this case would be Scipy.signal.

So the xtensor package shoud be xtensor-signal right?

spectre-ns commented 2 years ago

Yes that's correct.

JohanMabille commented 2 years ago

Here you go: https://github.com/xtensor-stack/xtensor-signal ;)

JohanMabille commented 2 years ago

I can setup a basic cmake and CI later (probably tomorrow)

spectre-ns commented 2 years ago

Awesome. Yes CI and CMake would be helpful as it would be nice to have consistently between repos. My CMake skills are also less than stellar.

spectre-ns commented 2 years ago

@JohanMabille I will leave this open until CI gets added. I can start adding PRs / issues in the other repo once the CI is in.

JohanMabille commented 2 years ago

Sure, sorry for the delay, I am swamped with the xeus stack. Will try to add it tomorrow.

spectre-ns commented 2 years ago

No worries! :) You guy's are working miracles on several fronts.