find-peaks icon indicating copy to clipboard operation
find-peaks copied to clipboard

endpoint bug

Open manuelbauer1603 opened this issue 5 years ago • 11 comments

Code throws an 'vector out of range' error, when there is a peak on an endpoint. Some examples: Signal: { 0, 0, 0, 1 } --> error Signal: { 0, 0, 1, 1 } --> no error, peak at pos 2 (seem wrong, there is no peak) Signal: { 0, 1, 0, 1 } --> error Signal: { 1, 0, 0, 0 } --> no error, no peaks (if code is supposed to ignore endpoint peaks, response is correct) Signal: { 1, 1, 0, 0 } --> no error, no peaks (seems correct) Signal: { 1, 0, 1, 0 } --> error Signal: { 1, 0, 0, 1 } --> error

How is the code supposed to handle endpoint peaks? Is there an easy fix to the errors?

Thanks.

manuelbauer1603 avatar Mar 06 '19 16:03 manuelbauer1603

I have also a problem when the maximum is at the end of the vector. If we take the provided example.cpp and put the maximum at the end:

float signal[14] = {0,1,1,1,1,1,1,5,1,1,1,1,1,7};

the returned vector of indexes is wrong:

5 9.10844e-44

h4k1m0u avatar May 18 '20 12:05 h4k1m0u

Does this error occur in the original Matlab code?

claydergc avatar May 19 '20 03:05 claydergc

No idea, sorry, I don't have Matlab installed on my computer.

h4k1m0u avatar May 19 '20 07:05 h4k1m0u

Matlab R2019b excludes endpoints (https://de.mathworks.com/help/signal/ref/findpeaks.html)

This is the Matlab output for the above mentioned cases:

Signal: { 0, 0, 0, 1 } --> no peak Signal: { 0, 0, 1, 1 } --> no peak Signal: { 0, 1, 0, 1 } --> peak at 2 Signal: { 1, 0, 0, 0 } --> no peak Signal: { 1, 1, 0, 0 } --> no peak Signal: { 1, 0, 1, 0 } --> peak at 3 Signal: { 1, 0, 0, 1 } --> no peak

manuelbauer1603 avatar May 19 '20 10:05 manuelbauer1603

I also have the same Problem. Any Idea how to prevent peak detection at the edges?

R-Fehler avatar May 26 '20 18:05 R-Fehler

I will check the code as soon as possible. If you can find the error, feel free to send a pull request. Remember that my code is a translation of the original code in Matlab. So, you could start by checking the Matlab code.

claydergc avatar May 26 '20 22:05 claydergc

Hey guys! Firstly, thanks for the C++ version, really great peak finder. Secondly, I too encountered this bug. Sometimes the last index is exactly the size of the input array.

The quick fix is easy, before you return do:

if (peakLocTmp[peakLoc.back()] >= x0.size())
            peakLoc.pop_back();

Just to be on a safe side you can

for (auto p : peakInds)
        assert(p < x0.size());

Asserts will be removed in non-debug builds.

I don't know how to make a proper fix in the algorithm itself though.

IvoryTowerDSPWizard avatar Jun 02 '20 08:06 IvoryTowerDSPWizard

I just updated the code. I fixed the bugs reported by @h4k1m0u and @manuelbauer1603. Additionally, I made the findPeaks function more similar to the original code. So, now it accepts more parameters like the code below:

findPeaks(std::vector<float> x0, std::vector<int>& peakInds, bool includeEndpoints=true, float extrema=1)

I hope this code can be useful for your purposes. Let me know if you find other bugs.

claydergc avatar Mar 13 '21 04:03 claydergc

@R-Fehler now, I added the parameter includeEndpoints. Just set it to false.

claydergc avatar Mar 13 '21 04:03 claydergc

not all peaks are detected, am I missing something?

image

k-maheshkumar avatar Aug 30 '22 15:08 k-maheshkumar

@claydergc First of all thank you for providing your code. I am a little late to the discussion. However, I think to get the endpoints correctly you have to change :

https://github.com/claydergc/find-peaks/blob/master/PeakFinder.cpp#L99-L101

to

		ind.insert(ind.begin(), 0);
		ind.insert(ind.end(), len0-1);

This should fix the problem of including the endpoints correctly, otherwise, I was off by one at both ends.

Feel free to correct me. Furthermore, in your example, the comment-out array contains 13 elements besides stating 14.

https://github.com/claydergc/find-peaks/blob/master/example.cpp#L6

daimmig avatar Dec 01 '23 17:12 daimmig