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

A question about most local maxima #6

Open
skyan opened this issue Aug 24, 2020 · 3 comments
Open

A question about most local maxima #6

skyan opened this issue Aug 24, 2020 · 3 comments

Comments

@skyan
Copy link

skyan commented Aug 24, 2020

Hi, @ig248 ,

I read the original paper and compared it with your implementation, I am wondering why below code reshapes LSM matrix by LSM[0:l_scale, :] but not by LSM[0:l_scale+1, :]?

pks_logical = np.min(LSM[0:l_scale, :], axis=0)

As the paper said, it reshapes the LSM matrix by removing all elements m_{k,i} for which k > λ holds. So it should lead to new λ ×N matrix. But in your implementation, the new matrix is (λ-1) ×N.

And under certain dataset, the l_scale may equals to 0, then LSM[0:0] will throw an exception...

Looking forward to your response

@OskarHofmann
Copy link

OskarHofmann commented Jan 21, 2021

Yeah I just downloaded this module and encountered the very same issue as l_scale is 0 for my data.
I also changed it to l_scale+1 and now it works.
The same problem is in line 52 for the original implementation:

pks_logical = np.min(LSM[0:l_scale, :], axis=0)

@yidilozdemir
Copy link

Just wanted to add here that, when I got an error message "zero-size array to reduction operation minimum which has no identity" from using find_peaks, implementing the change you suggested fixed my issue.

@code-yuan-shi
Copy link

Why when I try to change l_scale to l_scale+1 I get the wrong peak, although this does not report an error anymore.

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

No branches or pull requests

4 participants