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

mzR::get3Dmap only return zeros #269

Open
joerg-b opened this issue May 17, 2022 · 6 comments
Open

mzR::get3Dmap only return zeros #269

joerg-b opened this issue May 17, 2022 · 6 comments
Assignees

Comments

@joerg-b
Copy link

joerg-b commented May 17, 2022

I've recently updated my computer (macOS Monterey, R 4.2, Bioconductor 3.15) and in the process I've also installed mzR.
Now, while running some scripts that worked perfectly fine on R 3.6 and R 4.0, I'm encountering some problems. I've located the problem to originate from get3Dmap only returning a matrix of zeros. The dimensions of the matrix seem to be correct. When I use mzR::peaks on the same data file I get lot's of non-zero values.

I can provide a link to an example .mzML file if that helps.

Thanks for your help!
Joerg

@sneumann
Copy link
Owner

sneumann commented May 23, 2022

Hi, I can confirm, the following "test" fails:

library(mzR)
ms <- openMSfile(list.files(system.file("microtofq", package = "msdata"), pattern="MM14.mzML",full.names=TRUE, recursive = TRUE))
threedee <- get3Dmap(ms, scans=1:100, lowMz=100, highMz=1000, resMz=1)
sum(threedee) > 1

on

mzR_2.30.0
R version 4.2.0 (2022-04-22)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 20.04.4 LTS

It does work on

R version 3.6.3 (2020-02-29)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 16.04.7 LTS

@sneumann
Copy link
Owner

sneumann commented May 23, 2022

It is called from

setMethod("get3Dmap", "mzRpwiz",

Actual calculation happens in C++:

Rcpp::NumericMatrix RcppPwiz::get3DMap ( std::vector<int> scanNumbers, double whichMzLow, double whichMzHigh, double resMz )

My current suspicion is the updated pwiz version.

@sneumann
Copy link
Owner

So, some git bisect later:

2aed7135fee1fc23daa795b189bb62d335a3bbbf is the first bad commit
commit 2aed7135fee1fc23daa795b189bb62d335a3bbbf
Date:   Mon Oct 25 10:58:52 2021 +0200

        refactor: patially rewrite code to exctract header info
    
        - Create index list in R instead of C++.
        - Use `size_t` instead of `int` where suitable.
        - Ensure that correct data types are passed between R and C++.
        - Use new (own) implementation to extract the acquisition number from the
          spectrum ID instead of `id::translateNativeIDToScanNumber` from proteowizard
          to avoid segfaults on macOS (https://github.com/sneumann/xcms/issues/422).

 DESCRIPTION         |   2 +-
 NEWS                |   9 +++++
 R/methods-mzRpwiz.R |  10 +++---
 src/RcppPwiz.cpp    | 101 ++++++++++++++++++++++++++++++++++------------------
 src/RcppPwiz.h      |   8 +++--
 5 files changed, 89 insertions(+), 41 deletions(-)

@jorainer
Copy link
Collaborator

Uh - then it seems that's my fault. I'll have a look and fix.

@jorainer jorainer self-assigned this May 24, 2022
jorainer added a commit to jorainer/mzR that referenced this issue May 24, 2022
@jorainer
Copy link
Collaborator

Sorry, super-stupid misspelling from my side. Wrote DetailLevel_FullMetadata instead of DetailLevel_FullData in the spectrum call, thus only header information were imported but no m/z and intensity values. It's fixed now in PR #269 that also adds a unit test for the get3Dmap function.

sneumann added a commit that referenced this issue May 24, 2022
fix: read full spectra data in get3DMap (issue #269)
@sneumann
Copy link
Owner

Thank Jörg for reporting, thanks to git bisect run to identify the causal commit (my first guess was wrong), and Jo for the super fast fix. Pushed upstream as 2.31.1 Yours, Steffen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants