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

fabs creates binning bug when tolerance_in_ppm is True #6

Closed
RogerGinBer opened this issue Feb 21, 2024 · 1 comment
Closed

fabs creates binning bug when tolerance_in_ppm is True #6

RogerGinBer opened this issue Feb 21, 2024 · 1 comment

Comments

@RogerGinBer
Copy link

Hi Pere,

This fabs call in line 75 of the binning makes minMassDistance be always positive, which in turn makes the bin always insert after the current minDistanceIndex (even if the actual distance is negative)

if(tolerance_in_ppm)
{
minMassDistance = 1e6*(fabs(minMassDistance)/newMassBin.mass); //Compute distance in ppm
compTolerance = tolerance;
newMassBin.binSize = newMassBin.binSize < 0.0 ? (tolerance * newMassBin.mass / 1e6) : newMassBin.binSize; //Overwrite binSize with tolerance when no binSize data is available
}

rMSI2/src/peakbinning.cpp

Lines 101 to 110 in 08d7948

if(minMassDistance > 0)
{
//Insert the new mass channel after the minDistanceIndex
targetMassBins.insert(targetMassBins.begin() + minDistanceIndex + 1, newMassBin);
}
else
{
//Insert the new mass channel before the minDistanceIndex
targetMassBins.insert(targetMassBins.begin() + minDistanceIndex, newMassBin);
}

Running some debug tests, this clearly has a huge impact on the resulting data structures and total runtime:

image

@prafols
Copy link
Owner

prafols commented Apr 17, 2024

I've just checked it, I agree it was a mistake. Fixed

@prafols prafols closed this as completed Apr 17, 2024
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

2 participants