Interchanged pre and pos prefixes in peak_detection function

[jimearruti]

The prefixes were mixed up, as described in the linked issue. Now the filters are centered in the correct sample.

[mrocamora]

It's intriguing.

The code in librosa does exactly the same:

https://librosa.org/doc/latest/_modules/librosa/util/utils.html#peak_pick

As well as the code in madmom:

https://github.com/CPJKU/madmom/blob/main/madmom/features/onsets.py

Are they wrong? What do you think?

[jimearruti]

The result of both operations is then used as the origin parameter when calling sp.ndimage.filters.maximum_filter.

According to the documentation:
origin: int or sequence, optional
Controls the placement of the filter on the input array’s pixels. A value of 0 (the default) centers the filter over the pixel, with positive values shifting the filter to the left, and negative ones to the right.

I had read this before the PR but the direction of the shift had confused me. Reading this again (and drawing some examples for different windows) I now think it was okay before the change, and librosa and madmom are right.

What made you think they were interchanged? Maybe I'm missing something.

[mrocamora]

Look at the plot of the moving average and moving maximum thresholds in the onset detection demo notebook. They clearly extend longer to the future despite the fact that pre is greater than pos. Why is this happening? I thought the parameters were interchanged, but it is not clear to me now.

[jimearruti]

Yep, the plot indeed shows what you said, and is fixed if we interchange the prefixes. It's weird. Maybe we can talk it through in our next meeting, to make sure we're not missing anything.