MassBank / RMassBank

Playground for experiments on the official http://bioconductor.org/packages/devel/bioc/html/RMassBank.html
Other
12 stars 15 forks source link

merging issues + unit test for Electronic Noise #79

Closed schymane closed 9 years ago

schymane commented 10 years ago

Latest versions from Steffen and Michele don't match. Only example found so far: missing electronic noise removal in latest 1.5.2.2 from Steffen (Friday April 25), which is present in last 1.5.2.1 from Michele one month earlier (27 March). Please check if anything else is missing.

sneumann commented 10 years ago

The fix 9d5b7969e1f4035ce9dd291f3efd9c62a1563667 is present in https://github.com/sneumann/RMassBank/blob/master/R/leMsMs.r What else was required to fix the issue ?

schymane commented 10 years ago

Seems to be inconsistent performance and/or package install. Works now in a new R session other attached packages: [1] gplots_2.12.1 mzR_1.8.1 RMassBank_1.5.2.2 Rcpp_0.11.0

Investigate adding a unit test for noise removal using RMBData and default settings.

schymane commented 10 years ago

Re: unit test: see data in mail, sent 28/4/14 at 15:27.

ermueller commented 10 years ago

The problem with the unit test would be that XCMS doesn't differentiate between HCD and CID - that means we'd only get 7 spectra and not 14. Technically it works if I adjust the options - and the results look fine as a record, but building a unit test for records and molfiles is also pretty extensive.

Although - maybe there actually is an easy way out now that I think about it, and it may work just the same. I'll give you an update on that asap.

EDIT: The noise removal has nothing to do with XCMS anyways, sorry, forget about that.

meowcat commented 10 years ago

The problem with the unit test would be that XCMS doesn't differentiate between HCD and CID - that means we'd only get 7 spectra and not 14.

It is not the case that we have 7x HCD + 7x CID spectra - we have 2x6 HCD + 2x1 CID, at resolutions 7500 (first 7 spectra) and 15000 (second 7 spectra), at least in the LTQ Orbitrap XL records. The CID spectrum is easily differentiated because it is at NCE 35%. But I think the different resolutions is what XCMS can't differentiate. And actually, the mzR-based reader doesn't directly differentiate between the resolutions but implies the resolutions from the scan sequence provided in the spectraList option (i.e. the user is expected to write the spectraList corresponding to his scan setting he used in the Orbitrap setup.)

uchem-massbank commented 10 years ago

…and the idea with this unit test, I thought, was to test whether or not the electronic noise is removed correctly or not. To me, this would make most sense to do the normal mzR workflow that is run during the vignette, and see if those peaks turn up in the “fail peak” list or not. If they turn up there, they are not removed. The number of records is almost irrelevant for this test case, or am I missing something?

sneumann commented 10 years ago

Just missing the unitTest for now, Erik has an idea how to do that

schymane commented 9 years ago

I thought this was long solved but apparently not - the electronic noise is currently NOT working in the latest BioC version of RMassBank. RMassBank_1.9.1.tar.gz and RMassBank_1.4.0 (what BiocLite installs by default). In contrast, seems to be fixed on latest github version: RMassBank_1.5.2.5.tar.gz

sessionInfo(): see pasted version in #99

All versions have the problem reported in #99

ermueller commented 9 years ago

Both unit tests added in aa2d600 made a mistake in there, fixed the mistake in e6eabab