Skip to content

Conversation

@StanFromIreland
Copy link
Contributor

#27744

Proposed new feature or change:

Convolution is said to be often seen in signal processing. It would be nice that this function provides an "out parameter" as the >ufunc functions do. Convolve operation does not really operate in an element-by-element fashion but when we are used to >'abuse' of the "out parameter' for array operations in loops (mainly to avoid unnecessary memory management), this is annoying >to not be able to write similar code for the convolve operation.

I have added an optional out parameter as per FromHub's idea.


# Convulate and store result
result = multiarray.correlate(a, v[::-1], mode)
out[...] = result
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main use of an out argument is to provide the output buffer, so no extra memory needs to be allocated. But here you do not do that (indeed, if out is not passed in, memory usage now doubles compared to how it was).

The logical way would be to instead change multiarray.correlate and give it an out argument. Unfortunately, that means coding the lot up in C rather than python...

@FromHub
Copy link

FromHub commented Nov 20, 2024

Hi Stan,
I’m glad you’ve addressed my issue and have already begun the work! You’ve completed the first part, but please remember that the ultimate goal is to provide the out parameter to eliminate the creation of the result array within the call. This means propagating the out parameter to the underlying C function correlate to bypass "_ret = new_array_for_sum(ap1, ap2, NULL, 1, &length, typenum, NULL);".
From my quick insight into the multiarraymodule.c file, I see no valid reason why this feature is not yet available.

One of Python's advantages is that adding a parameter with a default value does not break the API. However, for the C API...
I do not know how many files call this internal function or what the restrictions are for making such an update.
At least

So in my opinion this 2 lines are not acceptable:
result = multiarray.correlate(a, v[::-1], mode)
out[...] = result
as it will not solve what is expected by the community (out parameter to bring no memory allocation, no copy).

As pointed by mhvk (he answer faster than me !), the work shall be made wider to englobate convole/correlate/correlate2, including the C part and the documentation...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants