search

rickhelmus / patRoon

Workflow solutions for mass-spectrometry based non-target analysis.
https://rickhelmus.github.io/patRoon/
GNU General Public License v3.0
65 stars 19 forks source link

Add qAlgorithms find features #116

Open ricardo-cunha opened 1 month ago

ricardo-cunha commented 1 month ago

Hi Rick,

so, here the first pull request to initiate revisions ;)

Cheers, Ricardo

rickhelmus commented 1 month ago

Hi Ricardo,

Many thanks!! :-) I'll try to find some time to review your changes. For now, I wanted to ask if you could try to clean up the commits a bit? I think it's easiest if you rebase your fork, see e.g. https://stackoverflow.com/a/33504534

Thanks, Rick

ricardo-cunha commented 1 month ago

I rebased and 6 commints were ignored, is as you expected? Let me know what you think. Regarding the direct integration of the cpp from qAlgorithms, I agree and tried but I was not able to compile with GCC from RTools somehow.

dahoehn commented 1 month ago

Hi Ricardo, hi Rick,

If you used a version of the main branch before the latest commit (1bc3c7a08e8d1bc5f3c4a770d268049faf0cb1dd), the code was either outdated or not functional. The program now compiles correctly on windows and linux (at least for me).

Just a quick note regarding the default settings: I strongly recommend to change the assumed centroid error from five ppm to 0.25 ppm. 0.25 is still a fairly arbitrary number, but roughly matches the centroid error we observed in our reference data. If a feature is not clearly detected, it tends to have a larger error up to ~0.3 ppm. The main problem here is that we only worked with orbitrap data so far. It is also by no extent a final estimate, some changes we are considering for the binning algorithm could change it significantly. We chose five in the beginning just to have a number, but it vastly overestimates the error and leads to bad results. I'd suggest adding a warning message along the lines of "there is a difference between centroid error and mass tolerance" when a value above 1 is set.

Thanks for implementing our work, Daniel

rickhelmus commented 1 month ago

Hi Daniel,

Thanks for the heads-up!