sneumann / xcms

This is the git repository matching the Bioconductor package xcms: LC/MS and GC/MS Data Analysis
Other
180 stars 80 forks source link

.centWave_orig - unexpected code #522

Open RonzoniInstitute opened 3 years ago

RonzoniInstitute commented 3 years ago

The following code in .centWave_orig is strange because it always returns 1:

w <- min(1:length(x),ncol(wCoefs))

jorainer commented 3 years ago

Indeed. I guess it should be min(length(x), ncol(wCoefs)), but I'm very reluctant to change anything in the original centWave implementation. @sneumann might have more insights into that function - what do you think? Should we change that?

sneumann commented 3 years ago

This is about the code in https://github.com/sneumann/xcms/blob/ae88b2e866fa8de8d1b900a2a738a1bb8a88f9b2/R/do_findChromPeaks-functions.R#L438 and https://github.com/sneumann/xcms/blob/ae88b2e866fa8de8d1b900a2a738a1bb8a88f9b2/R/do_findChromPeaks-functions.R#L3379 I haven't wrapped my mind around what it might have been intended to do :-( Yours, Steffen

RonzoniInstitute commented 3 years ago

This is my understanding of where this code is in the case of the .centWave_orig function: we have the ROI and for each ROI the CWT of the corresponding (extended) EIC has been computed. The local maxima and the ridges of local maxima of the CWT has been determined. We are at the point where we want to dermine the peaks of the current ROI from the ridges of the CWT. The code:

wpeaks <- sapply(rL, function(x) { w <- min(1:length(x),ncol(wCoefs)) any(wCoefs[x,w]- baseline >= sdthr) })

seems to check which of the ridges of the CWT contain signal above the signal-to-noise threshold (this is my opinion). In fact in the subsequent code one of these signal containing ridges detemines a peak if it has retention times in the (not extended) ROI rt-range and if the EIC at the ridge retention times has signal above the signal-to-noise threshold.

Not relevant, but I'm personally interested in the orginal centwave because the findChromPeaks method for signature object="OnDiskMSnExp" and param="CentWaveParam" seems to call .centWave_orig in XCMS 3.10.1.