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

bug in bin_Spectrum #137

Closed
jorainer opened this issue Jul 28, 2016 · 5 comments
Closed

bug in bin_Spectrum #137

jorainer opened this issue Jul 28, 2016 · 5 comments
Labels

Comments

@jorainer
Copy link
Collaborator

I think bin_Spectrum (https://github.com/lgatto/MSnbase/blob/master/R/functions-Spectrum.R#L296-L327) calculates one value too much:

  • The breaks span the range from floor(min(mz(object))) to ceiling(max(mz(object))).
  • These breaks define the bins and values falling within bins [x, x+1] should be summarized with fun.
  • The new vector of intensities is initialized with intensity <- double(length(breaks)) but that's one value too much. The breaks define the start and end value for each bin, and the result should be the intensity values averaged within each bin. To my understanding intensity should thus be intensity <- double(length(breaks)-1), i.e. one value for each bin, but not for each break.
  • The new mz values could then be calculated as mz <- breaks[-nb]+breaks[-1L])/2L.
@jorainer jorainer added the bug label Jul 28, 2016
@lgatto
Copy link
Owner

lgatto commented Aug 5, 2016

@sgibb - this looks indeed correct. Could you check too.

@jorainer
Copy link
Collaborator Author

jorainer commented Aug 5, 2016

@sgibb @lgatto as a info: I'm presently implementing also a generic binning function in xcms. it's in C and thus very fast. It allows to select the min, max, sum or mean value within each bin. I'm also investigating in ways to interpolate values for bins without values.

I've planned to replace with that function some or all of the profBinxxx methods that are already implemented in xcms but that are not so flexible to use. In the end that might be also a candidate functionality for a MS basic methods package.

@sgibb sgibb closed this as completed in a942daa Aug 5, 2016
@sgibb
Copy link
Collaborator

sgibb commented Aug 5, 2016

The behaviour was indeed a little bit strange.

The breaks [x, x+1] define a bin. A bin is created for x <= mz < x + 1. A special case arises if the highest break is identical with the highest mz value. That was the reason for adding an additional mz/intensity value at the RHS. Now this would be done only for this particular case.

@lgatto
Copy link
Owner

lgatto commented Aug 5, 2016

@sgibb I changed the unit test (see c80e161) because it failed (just a matter of last value mismatch).

@sgibb
Copy link
Collaborator

sgibb commented Aug 6, 2016

@lgatto thanks; Sorry, I just tested the bin_Spectrum part because most of the other test fail on my machine (because I still not completely installed all dependencies).

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

No branches or pull requests

3 participants