-
Notifications
You must be signed in to change notification settings - Fork 33
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
Above Averaging Algorithm Rework and Crash Fix #739
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nbollis
changed the title
Averaging Algorithm Rework and CrashFix
Averaging Algorithm Rework and Crash Fix
Nov 15, 2023
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #739 +/- ##
==========================================
- Coverage 76.08% 74.40% -1.68%
==========================================
Files 171 171
Lines 28876 28862 -14
Branches 0 2862 +2862
==========================================
- Hits 21969 21474 -495
+ Misses 6907 6902 -5
- Partials 0 486 +486
|
nbollis
changed the title
Averaging Algorithm Rework and Crash Fix
Above Averaging Algorithm Rework and Crash Fix
Nov 15, 2023
trishorts
approved these changes
Nov 15, 2023
elaboy
approved these changes
Nov 21, 2023
Alexander-Sol
approved these changes
Nov 22, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Crash was caused by MS1 and MS2 scans being out of order. The previous algorithm mistakenly added duplicates of the same MS2 scan, leading to a crash on writing the mzML file as the scan numbers were not unique.
This PR changes how averaging DDA scans works by first removing the overlap parameter. The algorithm will iterate through all MS1 scans, adding the number to be averaged to a local collection. The collection will be averaged, the first scan removed, and the next scan inserted before restarting the loop. All unaveraged scans (leading or lagging MS1s or all MSn+) will then be added at the end.
Due to this change in the algorithm, the tests had to be reconsidered.
Test now ensure that the same number of MS1 and MS2 scans are present in the averaged spectrum, that they are in the same order, and that their retention times are correct after averaging.
Converting the averaged MzSpectrum into a MsDataScan was excised to a helper method to clean up the algorithmic methods.
This PR was packed and added to MetaMorpheus. The accompanying MetaMorpheus PR is #2318 and will need to be slightly amended once this version of Mzlib is released