sneumann / mzR

This is the git repository matching the Bioconductor package mzR: parser for netCDF, mzXML, mzData and mzML files (mass spectrometry data)
42 stars 26 forks source link

Rewrite the pwiz header and peaks functions #207

Closed jorainer closed 5 years ago

jorainer commented 5 years ago

Rewrite the C++ code extracting header (issue #205) and peak information from mass spec files for the pwiz backend.

Tests and performance improvements for the new header implementation are shown in issue #205.

Tests and performance comparisons of the new C++ code for peaks is below. The main differences to the old implementation are that 1) the for loop is moved down to C++ and 2) one for loop within the old C++ code was avoided. No real great performance improvements, but IMHO it simplifies the (C++) code.

library(mzR)
library(microbenchmark)
library(testthat)

fl <- system.file("TripleTOF-SWATH/PestMix1_SWATH.mzML", package = "msdata")
msd <- openMSfile(fl)

pks <- mzR:::.peaks(msd)
pks2 <- mzR:::.peaks2(msd)
expect_identical(pks, pks2)

idx <- c(13, 17, 45)
pks <- mzR:::.peaks(msd, idx)
pks2 <- mzR:::.peaks2(msd, idx)
expect_identical(pks, pks2)

microbenchmark(mzR:::.peaks(msd),
               mzR:::.peaks2(msd),
               times = 10)
## Unit: seconds
##                expr      min       lq     mean   median       uq      max neval
##   mzR:::.peaks(msd) 1.461021 1.494350 1.522580 1.522239 1.540173 1.620640    10
##  mzR:::.peaks2(msd) 1.147891 1.231534 1.256024 1.252232 1.286919 1.382782    10

microbenchmark(mzR:::.peaks(msd, idx),
               mzR:::.peaks2(msd, idx),
               times = 50)
## Unit: microseconds
##                     expr   min    lq    mean median    uq   max neval
##   mzR:::.peaks(msd, idx) 486.5 490.3 502.340  493.2 497.1 707.4    50
##  mzR:::.peaks2(msd, idx) 420.6 422.2 443.462  423.8 427.1 702.3    50

close(msd)

Note: the .peaks call above uses the original implementation, the .peaks2 the updated code. In the PR .peaks is replaced by .peaks2.

jorainer commented 5 years ago

@lgatto, @sneumann, please have a look at this when you find the time.

lgatto commented 5 years ago

I'm ok with this and happy to merge and commit to Bioc is needed.

jorainer commented 5 years ago

It should be safe to merge I believe - if there are problems we still have enough time to fix. So, from my side, please merge and commit to Bioc - and let's keep the fingers crossed :)

lgatto commented 5 years ago

Done.