Thanks for the PR, I just enabled CI. If it fails you may need to enable CI on your fork to get CI passing in a timely manner.

I don't have time to go through the backlog of discussion about this and am not registering an opinion about whether we want to add this or not. That said, generally NumPy is pretty conservative about adding new keywords and functions, since the API surface is already very large and complicated.

One of the rules we have is that new features require sending a message to the numpy-discussion mailing list with a proposal to add the new feature. That gives the wider contributor base who don't actively follow github a chance to weigh in.

We're also generally conservative about adding new things to the C API, so your proposal to add a new C API item needs some justification. Keep in mind that basically no one will be able to use the NumPy 2.6 C API until perhaps 2028 due to projects keeping support for older NumPy releases. Is it really worth it to add the new C API? You'll need to explain why we should add the function.

Would you mind sending such a message to the mailing list about this proposal?

Thanks. I did post about this on the numpy-discussion list at the time (see https://mail.python.org/archives/list/numpy-discussion@python.org/thread/JMXNOCFQRFJYISFKSRI7MRL7GPK5ZD4S/#JMXNOCFQRFJYISFKSRI7MRL7GPK5ZD4S and especially https://mail.python.org/archives/list/numpy-discussion@python.org/thread/6FC27XPOD4SBH3KJ7VY5DDAPS5Q46GPO/#6FC27XPOD4SBH3KJ7VY5DDAPS5Q46GPO where several people chimed in saying this would be useful). I figured that was sufficient, but can re-post since it's been a while. You can also see several people hopping in to support the feature on the previous PR and closed issue.

As for the C API question, I believe it needs a new C API function because the core vector calculation implementation needs to be in C and the existing arguments are not sufficient to support the new implementation. Feel free to let me know if there is another way for the python function to call a modified version of the existing C functions.

I figured that was sufficient, but can re-post since it's been a while. You can also see several people hopping in to support the feature on the previous PR and closed issue.

Ah, I didn't search the archives. I'll take a look at those discussions and let you know if I think a new mailing list ping is warranted.

As for the C API question, I believe it needs a new C API function because the core vector calculation implementation needs to be in C and the existing arguments are not sufficient to support the new implementation.

But why does the new function need to be publicly exposed in the C API? Can't NumPy just call an internal C function?

Feel free to let me know if there is another way for the python function to call a modified version of the existing C functions.

You could not expose it publicly and instead just use the new C implementation internally.

oh sure, I didn't realize limiting it to internal use was an option. I don't know who uses the public C API. I guess the value of having the public C API function would just be to expose the functionality to whoever uses the C API as long as we are building the functionality anyway. But I certainly wouldn't want the question of the C API to delay the inclusion of the functionality in the python API

Oh great, good it was just a misconception. We definitely add features to the Python API without adding equivalent features in the public C API.

I just took a look and given that it’s been more than ten years since the mailing list discussion, I do think a new ping is warranted. When you send your message, please include context about prior discussions and how this proposal differs from earlier proposals, if it does.

Also, I haven’t checked but it would also be good to see what other array libraries do and what the array API standard says about this case. If adding this makes us more conformant with the array API or otherwise makes it easier to write code that is agnostic to the underlying array API, that would also argue towards including this.

However, if this is an entirely new feature that only NumPy would implement, that makes it a harder sell.

Also please feel free to come to a NumPy community meeting to chat face-to-face about this over Zoom:

https://scientific-python.org/calendars/

Ok, I will send an email to the list, and I will try to attend this week's zoom.

What do you mean by "other array libraries" and "array API standard"?

Array API Standard. In particular, note the Stakeholders which are those other array libraries.

convolve and correlate are not currently part of the standard, but those other libraries often do implement numpy functions that aren't in the standard yet. Changes to our implementation do have a "blast radius" in that it will either cause those libraries to do more work to follow numpy's choices or suffer greater divergence from numpy.

You may want to consider the option to implement more specialized functions in scipy.signal instead of modifying np.convolve and np.correlate. In particular, you could avoid needing returns_lagvector: we do not want to use this pattern of optional returns if we can at all avoid it. We will go to great lengths to avoid it.

Thanks for the pointer.
As you mention, convolve and correlate are not part of that standard, so it shouldn't break anything. The change I propose is backwards compatible so it will only cause more work for other libraries if they want to include this additional feature (which uses optional arguments). In that case it will be less work for them, as they only have to pass through instead of implementing from scratch themselves. I didn't find array libraries with conflicting implementations.
I found several related implementation ideas:

• Neural network libraries like pytorch and mxnet rely on a stride parameter for their convolution functions to avoid calculating convolutions at every lag (e.g. https://docs.pytorch.org/docs/stable/generated/torch.nn.modules.conv.Conv1d.html), similar to the stride entry in the range construction for the lags parameter proposed here.
• Matplotlib already has a maxlags argument on its xcorr function:. https://github.com/matplotlib/matplotlib/blob/dde076379dad6a51374625dbaa729a71958d4d88/lib/matplotlib/axes/_axes.pyi#L218. It actually calls numpy.correlate('full') and just throws out the extra entries, for a huge amount of wasted calculations. Numpy implementing this PR would allow matplotlib to immediately reap the performance benefits on their existing function parameters.
• Scipy has a correlation_lags function to calculate the lags for a given correlation mode after the fact, similar to the returns_lagvector optional argument here.
• The scipy people seemed to want this to be implemented here first and then pulled in over there when this was discussed at the time (Add maxlag kw for limited cross correlation window size to correlate/convolve functions scipy/scipy#4940 (comment))

If you want, we can remove the returns_lagvector argument. That was a suggestion proposed in the first round of getting feedback on this PR ten years ago. We could have a separate function to calculate lags like scipy has if you prefer.

I pushed a commit to remove the returns_lagvector argument and instead have a companion function like scipy has.

As you mention, convolve and correlate are not part of that standard, so it shouldn't break anything. The change I propose is backwards compatible so it will only cause more work for other libraries if they want to include this additional feature (which uses optional arguments).

That's not quite the logic at play here. It's not like a dependency (e.g. scipy depends on numpy, if a new optional argument is added in a backwards-compatible fashion, that's fine because all of the existing usages of np.convolve() inside of scipy still work). Adding a new optional argument would still affect any of these libraries that provide a convolve() or correlate() that attempt to follow numpy's current signature. That's because if numpy users use the new arguments, that code will not be portable to those array implementations by swapping out np for xp (the placeholder name for the other-array-library namespace). So those other implementations still face the choice to do the work to add the new feature or else live with the growing discrepancy. There's a cost either way, and there's nothing about the optionality or backwards compatibility of the new feature that alleviates that cost.

Thanks for the comments and the meeting today.

With respect to downstream libraries wanting to match numpy's signatures, I realized that it would be trivial for other libraries to accept a new lags/maxlag keyword and just compute the same complete correlation they currently do and then just slice it at the end and return the sliced version. That would keep the other libraries API consistent with numpy's very easily. They can then decide later whether they want to actually implement the performant version of lag subsetting or not.

Another point that came from the meeting today was with respect to interest in the feature. The fact that matplotlib has a maxlag keyword in its cross correlation function (that just slices at the end without any performance improvement) is evidence that there is interest in this functionality, and the fact that they call numpy.correlate means that it would be extremely easy for them to update their call to immediately take benefit of the performance improvement.
Additionally, I pointed out that there have been periodic comments on the previous closed PR expressing interest in the feature and asking when it would be implemented.

tasks for me from the numpy meeting today:

• Remove PyArray_CorrelateLags from C API (done, see commits above)
• post on scipy https://discuss.scientific-python.org/c/contributor/12 to see what they think.
• post on the Jax repo to see how they would feel about the numpy function signature changing and the lift necessary to match it.

Just a few other data points of other libraries having to do weird workarounds to deal with numpy.correlate being extremely inefficient for large arrays where only some lag values are relevant:

• ENH: Improve convolution performance for Sparse variables bids-standard/pybids#411 pybids downsampled (i.e. threw out some of) their source data in order to hack numpy.correlate into calculating fewer lags.
• O(T^2) complexity of stattools.acf statsmodels/statsmodels#4135 in statsmodels is solved with another weird workaround

Thanks! I think the statsmodels and matplotlib examples are probably sufficient to justify adding a maxlags keyword argument (following matplotlib).

@bashtage hope you don't mind the ping here, in case you want to weigh in from the statsmodels side.

I'd still like to get opinions from SciPy and Jax as well.

This is a common need, I am in favor.

Great. I posted on scipy here: https://discuss.scientific-python.org/t/custom-lag-parameters-on-correlate-and-convolve-functions/2307

and jax here: jax-ml/jax#37112.

i also posted on matplotlib, and they said that they are not concerned with consistency with numpy on that function. https://discourse.matplotlib.org/t/custom-lag-parameters-on-numpy-correlate-and-convolve-functions/26226

Does anyone have contacts at jax or scipy? No one responded to either post.

I posted on scipy here: https://discuss.scientific-python.org/t/custom-lag-parameters-on-correlate-and-convolve-functions/2307

and jax here: jax-ml/jax#37112.

@mhvk do you think this is useful for astropy? I am a bit doubtful about common enough the use-cases at least for lags that shouldn't be using the FFT trick to begin with.
maxlags had the argument that matplotlib has it/uses it, which seems relevant (but should check with them).

@seberg - I don't know that it would be much use for astropy (the main reason for our convolve is to deal with missing data...), but I do think I would use this myself. On the keywords, I think you are right that the case for maxlag is stronger, as almost always one will want symmetric lags (and steps other than 1 would be rarer still).

That said, a great advantage of being able to pass in lags is that you have those lags at hand right after, if, e.g., you want to plot the result. Note that I'm not in favour for introducing a dedicated function to return the lags, but it does point to a real problem, that it is not always all that obvious what the lags actually are. Obviously, this can (and should!) be solved by mentioning it in the docstring, but, heck, if you pass in lags, you don't have to worry about it at all. I do think that if one is allowed to pass in lags, they should just be any array of int; there is no reason to limit it to a slice.

Anyway, not sure whether or not to go with one, the other, or both...

Implementation-wise, I like that the main function in C now just takes lag information, but wonder if it isn't better to do the creation of that information from the keywords in python. I'd also tend not to change the C API interface at all, just change an internal function and call that from python. (I didn't even know the C version existed, and would agree with the random website I ran into when I googled it; https://runebook.dev/en/docs/numpy/reference/c-api/array/c.PyArray_Correlate2).