Repository navigation
Move conv to Convolutions.jl or ConvolutionsBase.jl? #615
Description
Downstream I have a DSP extension just to overload conv:
https://github.com/JuliaArrays/InfiniteArrays.jl/blob/master/ext/InfiniteArraysDSPExt.jl
I wonder if moving conv out would be better. This could take two approaches:
- Move out all convolutions code to Convolutions.jl. Potentially, FFTW could be a weak dependency but that may be hard.
- Make a package that just defines
convwith no implementation called ConvolutionsBase.jl. This has the benefit that there is no FFTW.jl dependency.
Activity
martinholters commentedon Dec 17, 2024
Even if we came up with a dispatch where an extension could hook in to do frequency-domain convolution, I wouldn't be happy to see the result of a convolution change depending on whether FFTW is loaded, even if it is within the numerical noise floor.
Sounds easy, but might not be, as it should then also define (by documentation) the exact API. Probably including any dispatch hooks like
conv_axis_with_offset; would be too bad if one had to depend onDSPbecause one doesn't want to add a method toconv, but ratherconv_axis_with_offsetorconv_output_axesor whatever might become public API there.So maybe it's better to start with a Convolutions.jl package that tries to fulfill the need of any downstream users and provide useful hooks. Once that has stabilized, one can then think about moving the definition of the API to a ConvolutionsBase.jl package.
Maybe we should try to bring in more users of convolution to the discussion. E.g. multiplication of polynomials is basically convolution, but Polynomials.jl can't depend on DSP.jl because DSP.jl depends on Polynomials.jl. Would a Convolutions.jl package be helpful for Polynomials.jl, @jverzani? If so, what would you expect of the API?
wheeheee commentedon Dec 17, 2024
I think (among others) ApproxFunBase.jl and Korg.jl also use only
conv. Don't know if the maintainers are ok with being pinged?dlfivefifty commentedon Dec 17, 2024
I can speak for ApproxFunBase. It's fine to ping me 😂
jverzani commentedon Dec 17, 2024
Polynomialswould likely benefit from this and could work around any API chose, I would assume as the requirements are pretty minimal (convolution of two vectors or ntuples basically).dlfivefifty commentedon Dec 17, 2024
I think convolution of two
Float64vectors is the case where FFTW.jl is needed but I think the current proposal is we leave FFTW as a hard dependency.wheeheee commentedon Dec 18, 2024
oh man, I saw jishnub's icon and thought that was it 😅
Hm, currently we don't support convolution of ntuples. @martinholters would it be appropriate to add that?
martinholters commentedon Dec 18, 2024
Sure, why not? Maybe not in DSP.jl, but in Convolutions.jl, that sounds reasonable. For future reference: for short homogenous tuples, this works quite well:
We'd probably want to do some
@generatedmagic, but the above might still be useful for anelsepart of anif @generated.dlfivefifty commentedon Dec 18, 2024
I think there needs to be an obvious use case for
Tuple. When would one want it instead of anSVector? Does a convolution make sense when the types are different?jverzani commentedon Dec 18, 2024
It comes up in
Polynomialsfor tuple-backed polynomials. But that might be quite niche, so needn't be in a more general package. I was just answering what was used, not necessarily, what was expected in an external package.dlfivefifty commentedon Dec 18, 2024
When do polynomials have coefficients of different types? Or are you using it instead of an
SVectorto avoid a dependency?jverzani commentedon Dec 18, 2024
The different types might occur, but primarily we have an option for using an ntuple which speeds up some operations (it was originally borrowed from https://github.com/tkoolen/StaticUnivariatePolynomials.jl). As surmised, an SVector is avoided just because of the dependency. Perhaps we can revisit that decision.
dlfivefifty commentedon Dec 19, 2024
I haven’t looked at how Polynomials.jl is implemented but supporting tuples seems like it must require a bunch of special cases, eg addition. I suspect it would be simpler to only allow
AbstractVectorcoefficients. It probably wouldn’t need to know anything about StaticArrays.jl,, eg, the special implementation ofconvmentioned would just be in a ConvulationsStaticArraysExt.jljverzani commentedon Dec 19, 2024
Fine with me. Thanks for the consideration.
hakkelt commentedon Jun 29, 2026
Hi there!
May I gently bump this conversation? As far as I see, @wheeheee created a #613, which paves the way for separating Convolutions.jl (which would depend on FFTW), and it wouldn't block the creation of an FFTW-free ConvolutionsBase.jl package later.
I use DSP only transitively, through Wavelets, and my motivation for this separation is different from @dlfivefifty : My problem is that Polynomials is quite a heavy dependency in terms of load time (on my laptop, DSP loads in 0.88 seconds, of which 0.84 seconds is spent loading Polynomials). And Wavelets only uses conv from DSP, so every time I launch a new session that imports Wavelets, I pay quite a high cost for loading code I don't use.
wheeheee commentedon Jul 15, 2026
So...given there are N=2 other people now who see a use for a Convolutions module, I suppose it's time to convert #613 from draft status.
For some reason though, it bloats test times (roughly double...) despite no real changes other than the submodule structure. All very strange.