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

Inconsistency in peak retention time correction of the retcor methods #122

Closed
jorainer opened this issue Feb 1, 2017 · 2 comments
Closed
Projects

Comments

@jorainer
Copy link
Collaborator

jorainer commented Feb 1, 2017

retcor.peakgroups and retcor.obiwarp perform both 1) the correction of the retention times per spectrum and 2) adjust the retention time reported for the identified features (peaks) in the @peaks slot.

The step 2) is performed in both methods with the code below (n being the number of samples), rtcor the raw retention times that are being corrected and rtdevsmo the difference between the raw and the corrected retention times. In other words, the difference by which the raw retention time have to be corrected.

    for (i in 1:n) {
        cfun <- stepfun(rtcor[[i]][-1] - diff(rtcor[[i]]) / 2,
                        rtcor[[i]] - rtdevsmo[[i]])
        rtcor[[i]] <- rtcor[[i]] - rtdevsmo[[i]]

        sidx <- which(corpeaks[,"sample"] == i)
        corpeaks[sidx, c("rt", "rtmin", "rtmax")] <-
            cfun(corpeaks[sidx, c("rt", "rtmin", "rtmax")])
    }

Now, retcor.obiwarp does something unexpected and incomprehensible to me:

        rtdevsmo[[s]] <- round(rtcor[[s]]-object@rt$corrected[[s]],2)

So, the .Call to obiwarp returns the adjusted retention times which are stored into object@rt$corrected, rtcor contains again the raw retention times at this stage. So, rtdevsmo is expected to be again the difference between the raw and the adjusted retention times, but why should we round this difference here?

This is rather strange to me, because that way we are correcting the peak retention times afterwards with something that does not exactly represent (mathematically speaking) the difference.

Now, I can live with or without the rounding because it will not cause a large difference in the results - but I would like to have it either in both or in none of the methods to be consistent.

For simplicity I would opt here to drop the round in the retcor.obiwarp, @sneumann , what do you think?

@jorainer jorainer added this to TODOs and ideas in 3.0.0 Feb 3, 2017
@jorainer
Copy link
Collaborator Author

The real puzzling and disturbing thing to me is that the old retcor.obiwarp method uses the (unrounded) adjusted rt as corrected rts for the spectra, but adjusts the rt of the identified peaks using rounded adjusted rt values. I would rather like to adjust the rt of the identified peaks also using the unrounded adjusted rt. @sneumann any comments or objections?

@jorainer
Copy link
Collaborator Author

jorainer commented Mar 7, 2017

This is now reported also in the new_functionality vignette.

@jorainer jorainer closed this as completed Mar 7, 2017
@jorainer jorainer moved this from TODOs and ideas to Done in 3.0.0 Mar 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
3.0.0
Done
Development

No branches or pull requests

1 participant