Skip to content

Initial implementation of quant mass manger for gcms project#657

Merged
Bujee415 merged 30 commits intomasterfrom
feature/quant-mass-manager
Oct 29, 2025
Merged

Initial implementation of quant mass manger for gcms project#657
Bujee415 merged 30 commits intomasterfrom
feature/quant-mass-manager

Conversation

@YukiMatsuzawa
Copy link
Copy Markdown
Contributor

No description provided.

This commit introduces two new static methods to the `ChromatogramRange` class: `FromTimes<T>(T center, double width)` and `FromTimes<T>(T begin, T end)`. The `FromTimes<T>(T center, double width)` method allows for creating a `ChromatogramRange` based on a center point and a specified width, while the `FromTimes<T>(T begin, T end)` method facilitates the creation of a `ChromatogramRange` using two defined endpoints.
Updated `GetMs1ExtractedChromatogram` to use a new
method `GetMs1ExtractedChromatogram_temp2` for improved
retrieval of MS1 extracted chromatograms. Added an
overloaded method that accepts an `MzRange` object,
enhancing the handling of mass-to-charge ratio and
tolerance parameters.
Introduced the `SearchNearestPoint` method in `Chromatogram.cs` to enhance peak searching functionality with improved handling of null peak lists. An overloaded version was also added for specific chromatogram types.

Refactored peak finding logic in `LcmsGapFiller.cs` to utilize the new `SearchNearestPoint` method, simplifying the code and improving readability. Adjusted `minId` handling to default to the middle of the chromatogram when no nearest peak is found.
Introduced `SearchLeftEdge`, `SearchRightEdge`, `SearchLeftEdgeHard`, and `SearchRightEdgeHard` methods in the `CompMs.Common.Components` namespace to enhance peak boundary detection based on intensity comparisons. Updated the `SearchNearestPoint` method to call a private overload with `ChromXs`.

Refactored the `LcmsGapFiller` class to utilize the new edge search methods, simplifying the logic for determining left and right indices around peaks. Updated peak top determination to use `IntensityDifference` for improved accuracy.
- Removed IReadOnlyList<RawSpectrum> parameter from
  AccumulateChromatogram in multiple classes to simplify
  method signatures and improve internal data handling.

- Updated PeakAligner.cs to reflect the new method
  signature by removing the spectrum argument.

- Added GetRepresentativeSpectrum method to Ms1Spectra
  for enhanced functionality in retrieving representative
  spectra.

- Optimized GetMSScanProperties methods to return lists
  directly from the chromatogram variable, improving
  performance.

- Adjusted using directives across several files for
  better organization and streamlined dependencies.

- Enhanced comments and documentation in the
  GetRepresentativeSpectrum method for improved
  readability and maintainability.
Introduce a static readonly instance of `ReportProgress` named `_empty` to streamline object creation when `reportAction` or `progress` is `null`. Update the `FromLength` and `FromRange` methods to return this instance instead of creating new `Progress<int>` objects, reducing unnecessary allocations and improving code consistency. These changes enhance readability and maintainability across multiple method overloads.
Introduces the `PeakQuantCalculation` class in the
`CompMs.MsdialGcMsApi.Algorithm` namespace. This class
facilitates asynchronous peak quantification calculations,
collects alignment peaks, and serializes spot information.
It leverages various data structures and algorithms from
the `CompMs` library, including temporary file handling
and progress reporting.
- Introduced `CreatePeakQuantCalculation` method in `GcmsAlignmentProcessFactory` for peak quantification.
- Added properties in `AlignmentFileBeanModel` to improve access to alignment file details.
- Updated `GcmsAlignmentModel` to accept `PeakQuantCalculation` for direct utilization in `QuantmassBrowserModel`.
- Modified `QuantmassBrowserModel` constructor to integrate quantification features.
- Enhanced `MainWindow.xaml.cs` to provide a "Run" action for quantification in the UI.
- Implemented `FinishCommand` in `QuantmassBrowserViewModel` to trigger quantification asynchronously.
Updated AlignmentFileBeanModel to include EicFilePath for retrieving the EIC file path. Removed CreateEicLoader method and replaced its usage with direct instantiation of AlignmentEicLoader across multiple files. Adjusted AlignmentEicLoader constructor to accept AlignmentFileBeanModel, modifying internal logic to utilize the new EicFilePath property.
Changed the `SelectedItem` binding in the `DataGrid` from
`{Binding Path=SelectedSpot.Value}` to `{Binding Path=Spots/}`.
This updates the selection logic to bind directly to the
`Spots` collection.
This commit introduces a loop that iterates over the
collection of `spots` and invokes the `SetRepresentativeProperty`
method from the `DataObjConverter` class for each `spot`.
This enhancement ensures that each `spot` has its representative
property set after collecting alignment peaks and reporting progress.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the gap-filling functionality in MS-DIAL, introducing a new IGapFiller interface and significantly refactoring the GC-MS and LC-MS gap-filling implementations. It also adds a new "Quant mass browser" feature for GC-MS alignment and removes unused parameters from method signatures across multiple data accessor classes.

  • Introduced IGapFiller interface to standardize gap-filling implementations
  • Refactored LcmsGapFiller and GcmsGapFiller to implement the new interface with standalone logic
  • Added a quantification mass browser UI feature for GC-MS projects
  • Removed the unused spectrum parameter from AccumulateChromatogram methods across all data accessors

Reviewed Changes

Copilot reviewed 49 out of 49 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
IGapFiller.cs New interface defining the gap-filling contract
GapFiller.cs Updated base class to implement IGapFiller
LcmsGapFiller.cs Complete refactor implementing standalone gap-filling logic
GcmsGapFiller.cs Refactored to implement IGapFiller with independent gap-filling implementation
DataAccessor.cs and implementations Removed unused spectrum parameter from AccumulateChromatogram
AlignmentSpotPropertyModel.cs Added QuantMass property with change notification
QuantmassBrowserModel.cs New model for quant mass browser feature
QuantmassBrowserView.xaml New UI for browsing and editing quantification masses
Chromatogram.cs Added utility methods for peak detection and edge finding
ChromatogramRange.cs Added FromTimes overload for center/width construction
Ms1Spectra.cs Added GetRepresentativeSpectrum and GetMs1ExtractedChromatogram overload

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

d:DesignHeight="450" d:DesignWidth="800">
<Grid>
<DataGrid ItemsSource="{Binding Path=Spots}"
SelectedItem="{Binding Path=Spots/}"
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The binding path Spots/ is incomplete. This should be SelectedItem=\"{Binding Path=SelectedSpot}\" to properly bind to the ViewModel's SelectedSpot property.

Copilot uses AI. Check for mistakes.
var peaklist = ms1Spectra.GetMs1ExtractedChromatogram(center.Mz.Value, this.mzTol, chromatogramRange);
return peaklist.ChromatogramSmoothing(smoothingMethod, smoothingLevel).AsPeakArray();
int id = nearestTopId;
for(int i = leftId + 1; i <= rightId - 1; i++) {
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space after for keyword. Should be for (int i = leftId + 1; i <= rightId - 1; i++) to follow C# coding conventions.

Copilot uses AI. Check for mistakes.

if (!isForceInsert && (id - leftId < 2 || rightId - id < 2)) return (-1, -1, -1);

for(int i = leftId + 1; i <= rightId - 1; i++) {
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space after for keyword. Should be for (int i = leftId + 1; i <= rightId - 1; i++) to follow C# coding conventions.

Suggested change
for(int i = leftId + 1; i <= rightId - 1; i++) {
for (int i = leftId + 1; i <= rightId - 1; i++) {

Copilot uses AI. Check for mistakes.
.Select(i => _spectra[i])
.Where(spectrum => spectrum.MsLevel == 2)
.Where(spectrum => spectrum.ScanPolarity == scanPolarity)
.Where(spectrum => spectrum.Precursor.ContainsMz(rtPeakFeature.PeakFeature.Mass, ms2Tolerance, acquisitionType)) // mz is in range
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Property access changed from rtPeakFeature.Mass to rtPeakFeature.PeakFeature.Mass. Verify that ChromatogramPeakFeature has a PeakFeature property with a Mass member, as this could cause a null reference exception if the property path is incorrect.

Suggested change
.Where(spectrum => spectrum.Precursor.ContainsMz(rtPeakFeature.PeakFeature.Mass, ms2Tolerance, acquisitionType)) // mz is in range
.Where(spectrum => rtPeakFeature.PeakFeature != null && spectrum.Precursor.ContainsMz(rtPeakFeature.PeakFeature.Mass, ms2Tolerance, acquisitionType)) // mz is in range

Copilot uses AI. Check for mistakes.
public interface IGapFiller
{
bool NeedsGapFill(AlignmentSpotProperty spot, AnalysisFileBean analysisFile);
void GapFill(Ms1Spectra ms1Spectra, RawSpectra rawSpectra, IReadOnlyList<RawSpectrum> spectra, AlignmentSpotProperty spot, int fileID);
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IGapFiller interface includes both RawSpectra and IReadOnlyList<RawSpectrum> parameters, which appears redundant since RawSpectra internally contains the spectrum collection. Consider removing the spectra parameter if it's not needed by all implementations.

Suggested change
void GapFill(Ms1Spectra ms1Spectra, RawSpectra rawSpectra, IReadOnlyList<RawSpectrum> spectra, AlignmentSpotProperty spot, int fileID);
void GapFill(Ms1Spectra ms1Spectra, RawSpectra rawSpectra, AlignmentSpotProperty spot, int fileID);

Copilot uses AI. Check for mistakes.
}

public AlignmentSpotPropertyModelCollection? Spots => _spots;
public AlignmentSpotPropertyModelCollection Spots => _spots ?? throw new ObjectDisposedException(nameof(Spots));
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Throwing ObjectDisposedException with nameof(Spots) is misleading because Spots is the property name, not the disposed object. Should use nameof(AlignmentSpotSource) or a more descriptive message to indicate the entire source has been disposed.

Suggested change
public AlignmentSpotPropertyModelCollection Spots => _spots ?? throw new ObjectDisposedException(nameof(Spots));
public AlignmentSpotPropertyModelCollection Spots => _spots ?? throw new ObjectDisposedException(nameof(AlignmentSpotSource));

Copilot uses AI. Check for mistakes.
return topId >= 1 && topId < _size - 1
&& _peaks[topId - 1].Intensity <= _peaks[topId].Intensity
&& _peaks[topId].Intensity >= _peaks[topId + 1].Intensity;
&& _peaks[topId].Intensity >= _peaks[topId + 1].Intensity
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The additional conditions for IsPeakTop (lines 575-576) exclude flat peaks where all three consecutive points have equal intensity. While this may be intentional, this logic change affects peak detection throughout the system and should be clearly documented or have a comment explaining why flat peaks are excluded.

Suggested change
&& _peaks[topId].Intensity >= _peaks[topId + 1].Intensity
&& _peaks[topId].Intensity >= _peaks[topId + 1].Intensity
// Exclude flat peaks: if all three consecutive points have equal intensity, do not consider as a peak top.

Copilot uses AI. Check for mistakes.
}
}

private readonly static ReportProgress _empty = new(null, 0d, 100d);
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The modifier order should be private static readonly instead of private readonly static to follow C# conventions (access modifier, then static, then readonly).

Suggested change
private readonly static ReportProgress _empty = new(null, 0d, 100d);
private static readonly ReportProgress _empty = new(null, 0d, 100d);

Copilot uses AI. Check for mistakes.
@Bujee415 Bujee415 merged commit 3972ff6 into master Oct 29, 2025
9 checks passed
@Bujee415 Bujee415 deleted the feature/quant-mass-manager branch October 29, 2025 12:10
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

Successfully merging this pull request may close these issues.

3 participants