Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

endpoint bug #2

Open
manuelbauer1603 opened this issue Mar 6, 2019 · 11 comments · May be fixed by #6
Open

endpoint bug #2

manuelbauer1603 opened this issue Mar 6, 2019 · 11 comments · May be fixed by #6

Comments

@manuelbauer1603
Copy link

manuelbauer1603 commented Mar 6, 2019

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.

@h4k1m0u
Copy link
Contributor

h4k1m0u commented May 18, 2020

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

@claydergc
Copy link
Owner

Does this error occur in the original Matlab code?

@h4k1m0u
Copy link
Contributor

h4k1m0u commented May 19, 2020

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

@manuelbauer1603
Copy link
Author

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

@R-Fehler
Copy link

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

@claydergc
Copy link
Owner

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.

@IvoryTowerDSPWizard
Copy link

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.

@claydergc
Copy link
Owner

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
Copy link
Owner

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

@adenyes adenyes linked a pull request Aug 12, 2021 that will close this issue
@k-maheshkumar
Copy link

not all peaks are detected, am I missing something?

image

@daimmig
Copy link

daimmig commented Dec 1, 2023

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants