Conversation
Encapsulated the AmplitudeCutoff parameter within ChromDecBaseParam to improve code organization and modularity. Updated all references to use param.ChromDecBaseParam.AmplitudeCutoff across multiple files and methods, including MSDecHandler.cs, ParameterBase.cs, and others. This refactoring enhances maintainability and readability without changing functionality. Developers must now access AmplitudeCutoff via ChromDecBaseParam, ensuring consistency across the codebase.
Introduced a new `RelativeAmplitudeCutoff` parameter to enable dynamic intensity thresholding based on a spectrum's maximum intensity. Updated logic across multiple files to calculate thresholds as the maximum of `maxIntensity * RelativeAmplitudeCutoff` and `AmplitudeCutoff`. Key changes: - Added `RelativeAmplitudeCutoff` to `ParameterBase`, `DeconvolutionSettingModel`, and `ParameterBaseVM`. - Updated spectrum filtering logic in `MSDecHandler.cs`, `DataAccess.cs`, and `Ms2Dec.cs`. - Enhanced configuration parsing in `ConfigParser.cs` to support the new parameter. - Improved code readability with shorthand syntax for empty lists and removal of magic numbers. These changes improve deconvolution flexibility and configurability, adapting filtering to spectrum intensity distributions and exposing the new parameter in the UI and configuration files.
- Added a new "MS/MS relative abundance cut off" input in `DeconvolutionSettingView.xaml` with labels, tooltips, and binding to the `RelativeAmplitudeCutoff` property. - Introduced `RelativeAmplitudeCutoff` in `DeconvolutionSettingViewModel.cs` as a validated `ReactiveProperty` with error handling and synchronization with the model. - Updated `ObserveHasErrors` and `ObserveChanges` to include `RelativeAmplitudeCutoff` for validation and change tracking. - Added validation attributes (`[Required]`, `[RegularExpression]`) for `RelativeAmplitudeCutoff` to ensure valid percentage input. - Introduced `RemoveAfterPrecursor` as a new property in the view model with synchronization and change observation. - Improved overall functionality and user experience for deconvolution settings with better validation and configurability.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces a new RelativeAmplitudeCutoff parameter to improve spectrum filtering by applying both relative and absolute intensity thresholds during deconvolution. Key changes include updating core deconvolution logic, adding UI support through the ViewModel and XAML view, and enhancing configuration file parsing.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/MSDIAL5/MsdialCoreTestApp/Parser/ConfigParser.cs | Updated parsing to route amplitude cutoff values to the new ChromDecBaseParam. |
| src/MSDIAL5/MsdialLcMsApi/Algorithm/Ms2Dec.cs | Modified threshold calculation to incorporate RelativeAmplitudeCutoff. |
| src/MSDIAL5/MsdialLcImMsApi/Algorithm/Ms2Dec.cs | Adjusted handling for empty spectrum lists and threshold computation. |
| src/MSDIAL5/MsdialImmsCore/Algorithm/Ms2Dec.cs | Updated centroid calculation and handling of empty lists. |
| src/MSDIAL5/MsdialGuiApp/ViewModel/Setting/DeconvolutionSettingViewModel.cs | Added reactive binding and validation for the new parameter. |
| src/MSDIAL5/MsdialGuiApp/ViewModel/DataObj/ParameterBaseVM.cs | Propagated RelativeAmplitudeCutoff into the ViewModel. |
| src/MSDIAL5/MsdialGuiApp/View/Setting/DeconvolutionSettingView.xaml | Integrated new UI elements for relative amplitude input. |
| src/MSDIAL5/MsdialGuiApp/Model/Setting/MethodSettingModelFactory.cs | Adjusted parameter assignment to use ChromDecBaseParam for amplitude cutoff. |
| src/MSDIAL5/MsdialGuiApp/Model/Setting/DeconvolutionSettingModel.cs | Added RelativeAmplitudeCutoff property and ensured its persistence. |
| src/MSDIAL5/MsdialDimsCore/Algorithm/Ms2Dec.cs | Revised spectrum threshold computation to use the relative cutoff. |
| src/MSDIAL5/MsdialCore/Utility/DataAccess.cs | Updated filtering logic on centroid spectra using the new cutoff. |
| src/MSDIAL5/MsdialCore/Parameter/ParameterBase.cs | Revised text output to include the new RelativeAmplitudeCutoff parameter. |
| src/MSDIAL5/MsdialCore/MSDec/MSDecHandler.cs | Modified deconvolution spectrum refinement and chromatogram extraction based on updated cutoff values. |
src/MSDIAL5/MsdialGuiApp/Model/Setting/DeconvolutionSettingModel.cs
Outdated
Show resolved
Hide resolved
…el.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Bujee415
approved these changes
May 9, 2025
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
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.
PR Classification
New feature
PR Summary
Introduces a
RelativeAmplitudeCutoffparameter for flexible spectrum filtering based on relative intensity thresholds, complementing the existing absolute threshold. Updates span core logic, UI, ViewModel, and configuration parsing for seamless integration.MSDecHandler.cs: Updated spectrum filtering logic to use both relative and absolute amplitude cutoffs.ParameterBase.cs: AddedRelativeAmplitudeCutoffproperty with default value and updatedToStringmethod.DeconvolutionSettingView.xaml: Added UI input forRelativeAmplitudeCutoffwith validation and tooltips.ConfigParser.cs: Enabled parsing ofRelativeAmplitudeCutofffrom configuration files.Ms2Dec.cs: Enhanced MS/MS spectrum handling to incorporate the new parameter in intensity threshold calculations.