pymc-devs / pymc-devs/pytensor
Use numpy Correlate2 for Convolve1d implementation
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 644
- Forks
- 208
- Avg merge
- 2d 14h
- Merged PRs (30d)
- 16
Description
Description
This is what numpy.convolve uses under the hood, but we can avoid the python overhead in the C-backend: https://numpy.org/devdocs/reference/c-api/array.html#c.PyArray_Correlate2
It may make sense to actually implement everything in terms of a correlate1d and have the convolve1dbe a call oncorrelate1dwith the flipped input. Since under the hood callingnp.convolve` is doing this flip anyway.
correlate1d shows up in the gradient of convolve1d (note the [::-1] inputs)
https://github.com/pymc-devs/pytensor/blob/afb7695110dec32e13247aadafd245639d96f7c4/pytensor/tensor/signal/conv.py#L68-L71
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with pytensor/tensor/signal/conv.py, especially the convolve1d gradient around lines 68–71, and trace the current C-backend implementation. Read NumPy’s PyArray_Correlate2 API and determine how the proposed correlate1d relationship fits the existing operations. Done means convolve1d uses the C-backed correlation path while preserving its current behavior and gradient.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- numpy, python
- Domain
- backend, performance
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100