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

In FNN::get.knnx(landmarklcms, idents, k = nlandmarks) k should be less than sample size #7

Open
EidrianGM opened this issue Jul 16, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@EidrianGM
Copy link

When running msPIP function there is a point where the error mentioned in the title occurs.

I was wondering if the following is a valid solution:

nlandmarks <- min(50,nrow(landmarklcms))
# 50 because is the default of nlandmarks in the msPIP function 

The idea is to select the whole set of landmarks available in case that they are less than 50 to find the maximum number of nearest neighbors.

@soroorh
Copy link
Contributor

soroorh commented Jul 16, 2024

thanks Adrian. I think you previously had other suggestions for msPIP. do you want to make a PR and i will have a look at your suggestions?

i think something like

nlandmarks <- min(nlandmarks, nrow(landmarklcms)) 

might be more robust/generic. but also have to have a closer look at the code!

Feel free to open a PR! thx

@EidrianGM
Copy link
Author

Yes that is indeed better implementation but opens a new question for me regarding the methodology, which might not make sense but since you know better how to treat the proteomics data I wonder

Would be there any issue/implications when using different number of nlandmarks for each run/sample ??

@soroorh
Copy link
Contributor

soroorh commented Jul 17, 2024

thanks for the question. msPIP in msImpute is very experimental and implementations were not fully evaluated. We are though experimenting with "number of landmarks" in a different but related project, and there we see it makes difference. But yes, generally given that the landmarks serve the purpose of between-run alignment, they should be shared across runs, where possible. but this i haven't tested in context of msImpute.

As i mentioned, feel free to suggest your changes in a PR. Happy to consider!

Hope this helps

@soroorh soroorh added the enhancement New feature or request label Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants