smith-chem-wisc / mzLib

Library for mass spectrometry projects
GNU Lesser General Public License v3.0
26 stars 33 forks source link

Binary search related bug in MzSpectrum.Extract #710

Open avcarr2 opened 1 year ago

avcarr2 commented 1 year ago

https://github.com/smith-chem-wisc/mzLib/blob/b0f82ea7bf47321ad2b81833961bff6d5706fe13/mzLib/MassSpectrometry/MzSpectra/MzSpectrum.cs#L718-L730

The linked method has an error commonly associated with performig binary searches on double arrays. The method as written uses a binary search to find the minimum mz value in the MzSpectrum.XArray and does not test to see if the correct, closest value is the index to the right or the left.

To fix this issue, replace lines currently binary search associated code should be replaced with the following: ` int ind = Array.BinarySearch(XArray, minX); if (ind < 0) { ind = ~ind; }

        if (ind >= XArray.Length)
        {
            ind = XArray.Length - 1;
        }

        if (ind != 0 && XArray[ind] - minX >
            minX - XArray[ind - 1])
        {
            ind--;
        }`
avcarr2 commented 1 year ago

https://github.com/smith-chem-wisc/mzLib/blob/b0f82ea7bf47321ad2b81833961bff6d5706fe13/mzLib/MassSpectrometry/Deconvolution/Algorithms/ClassicDeconvolutionAlgorithm.cs#L290C8-L311

Another probably incorrect implementation of handling binary search on double arrays.

avcarr2 commented 1 year ago

Here, starting index again doesn't correctly determine which index it is closer to. This one is more serious because it has 17 references. https://github.com/smith-chem-wisc/mzLib/blob/b0f82ea7bf47321ad2b81833961bff6d5706fe13/mzLib/MassSpectrometry/MzSpectra/MzSpectrum.cs#L673-L695