Fix bug where Skyline sometimes ignores the largest detected peak #3410
Conversation
This file contains hidden or 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
Fixed bug where largest detected peak might be ignored (reported by Erin)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
A user had a document where Skyline was not detecting the peak which was clearly the absolute largest one visible, and which also did not overlap with any of the other candidate peaks:
https://skyline.ms/announcements/home/support/thread.view?rowId=68758
The problem is that Skyline first decides how many peaks to keep based on how many peaks are significantly larger than the rest of the detected peaks. Skyline decided to keep 3 peaks.
Then, Skyline sorts all of the detected peaks by "Combined Score".
One of the smaller peaks that was slated to be rejected ended up with a higher Combined Score than the largest peak that had been detected.
There was code to make sure that any peak with a higher combined score than the initially big peaks would be kept, but I am pretty sure there was supposed to be a "+ 1" in there so that all of the peaks which were initially the largest are also kept, because otherwise this thing ends up happening.
Also, when writing the unit test I ran into the problem that the "has_midas_spectra" attribute is declared on the wrong element in the .xsd files.
This does not cause a failure in the MIDAS tests because a method called "UnflagFiles" ends up removing that attribute before the end of the test. I'll try to figure out what is going on with that.