IsoTracker performance improvement #942
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #942 +/- ##
==========================================
- Coverage 80.90% 80.90% -0.01%
==========================================
Files 265 265
Lines 38245 38269 +24
Branches 4129 4146 +17
==========================================
+ Hits 30943 30962 +19
- Misses 6605 6607 +2
- Partials 697 700 +3
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
Optimizes IsoTracker performance by improving parallelization and XIC extraction methods, achieving a 50% runtime reduction. The changes focus on removing inefficient parallelization in isobaric peak quantification, implementing efficient scan range-based XIC extraction, and using binary search for scan lookup.
- Removed outer parallel loop in isobaric peak quantification and optimized XIC alignment with better parallelization
- Added GetXicFromScanRange method for more efficient peak extraction across scan ranges
- Replaced linear search with binary search for scan lookup in BuildXIC method
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| mzLib/Test/FlashLFQ/TestIsoTracker.cs | Updates test assertions to use RtShift property and improves test data handling |
| mzLib/Test/FlashLFQ/IndexingEngineTests.cs | Adds comprehensive test coverage for new GetXicFromScanRange method |
| mzLib/MassSpectrometry/PeakIndexing/IndexingEngine.cs | Implements new GetXicFromScanRange method for efficient peak extraction |
| mzLib/FlashLFQ/IsoTracker/XICGroups.cs | Optimizes XIC alignment with improved parallelization and removes RTDict dependency |
| mzLib/FlashLFQ/IsoTracker/XIC.cs | Updates XIC alignment resolution and improves time calculation accuracy |
| mzLib/FlashLFQ/FlashLfqEngine.cs | Removes outer parallelization, optimizes BuildXIC with binary search, and improves ID comparison |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| Assert.AreEqual(xicGroup.RTDict.Count, 2); | ||
| Assert.AreEqual(xicGroup.RTDict[0], 0.0); //reference XIC Rt is 0 | ||
| Assert.AreEqual(xicGroup.RTDict[1], 0.0); //non-reference XIC Rt is 0 | ||
| Assert.AreEqual(xicGroup.XICs[1].RtShift, 0.0); //reference XIC Rt is 0 |
There was a problem hiding this comment.
Both assertions check the same XIC (index 1). The second assertion should check index 0 for the reference XIC, not index 1 again.
| Assert.AreEqual(xicGroup.XICs[1].RtShift, 0.0); //reference XIC Rt is 0 | |
| Assert.AreEqual(xicGroup.XICs[0].RtShift, 0.0); //reference XIC Rt is 0 |
Copilot uses AI. Check for mistakes.
| Assert.AreEqual(peptidesList.Count, isobaricPeptideNum); | ||
| Assert.AreEqual(peaksList.Count, isobaricPeakNum); | ||
| List<string> expectedSequence = new List<string> { "DIVENYFM[Common Variable:Oxidation on M]R", "DIVENYF[Common Variable:Oxidation on M]MR", "DIVENY[Common Variable:Oxidation on M]FMR" }; | ||
| expectedSequence.Sort(); |
There was a problem hiding this comment.
Sorting the expectedSequence list modifies the original list, which could affect subsequent test runs if this list is reused. Consider creating a sorted copy instead.
| expectedSequence.Sort(); | |
| expectedSequence = expectedSequence.OrderBy(s => s).ToList(); |
Copilot uses AI. Check for mistakes.
| @@ -1,8 +1,8 @@ | |||
| using MzLibUtil; | |||
| using MathNet.Numerics.RootFinding; | |||
There was a problem hiding this comment.
The imported MathNet.Numerics.RootFinding namespace appears to be unused in this file. Consider removing it if it's not needed.
| using MathNet.Numerics.RootFinding; |
Copilot uses AI. Check for mistakes.
| /// <param name="idList2"></param> | ||
| /// <returns></returns> | ||
| private bool IDsEqual(List<Identification> idList1, List<Identification> idList2) | ||
| public bool IDsEqual(List<Identification> idList1, List<Identification> idList2) |
There was a problem hiding this comment.
Changing the method visibility from private to public and sorting the input lists in-place can cause side effects for callers. The method modifies the original lists, which could break other code that expects the lists to remain unchanged.
Copilot uses AI. Check for mistakes.
Optimized parallelization to increase the speed of IsoTracker. This lead to a 50% decrease in total runtime when running IsoTracker + MBR on 24 files.
When tested on 24 files, most of the performance increase comes from optimizations to the AddIsoPeaks method. However, when tested on a large number of files (370), performance increased by...