Skip to content
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

IsoDec incorporation #2442

Open
wants to merge 51 commits into
base: master
Choose a base branch
from

Conversation

nbollis
Copy link
Member

@nbollis nbollis commented Nov 21, 2024

Added Isodec deconvolution to MetaMorpheus.
Allowed customization of MS2 deconvoltuion parameters.

Isodec Decon can be switched out for classic decon in the GUI without the Search Task needing to know which decon type it is using.

PR Summary

Enhances deconvolution parameter handling by making them serializable, adding new UI controls, and improving view model management.

  • CommonParameters.cs: Removed [TomlIgnore] attribute from DeconvolutionParameters properties to ensure decon params get saved with each task ran
  • Added IsoDecDeconParamControl user control and updated HostDeconParamControl.xaml for new UI elements.
  • DeconHostViewModel.cs and DeconParamsViewModel.cs: Updated to manage IsoDecDeconvolutionParameters.
  • Updated test files to include tests for IsoDecDeconvolutionParameters and related view models.

nbollis and others added 30 commits May 21, 2024 15:08
* Updated to MzLib 1.0.548 and fixed custom ions in search tasks

* reverted calibration task change

* merged in master bbbyy

* Enabled Library Loading from command line
Streamlined deconvolution parameter management by introducing `DeconHostViewModel` and custom control `HostDeconParamControl`. Updated `CommonParameters.cs` to change default `ProductDeconvolutionParameters` in negative mode from `-10` to `-20`. Modified `SearchTaskWindow.xaml` and `SearchTaskWindow.xaml.cs` to use the new view model, improving separation of concerns and maintainability. Updated `SaveButton_Click` and other relevant methods to retrieve parameters via the view model.
… set from the GUI with the new code structure
Refactored several XAML files to improve UI structure and readability:
- Removed unnecessary `GroupBox` in `SearchTaskWindow.xaml` and added a new one for "Peak Trimming".
- Adjusted layout in `ClassicDeconParamsControl.xaml` and `HostDeconParamControl.xaml`.

Enhanced ViewModel logic:
- Updated `DeconHostViewModel.cs` and `DeconParamsViewModel.cs` to include additional properties and validation logic.
- Added `[ExcludeFromCodeCoverage]` attribute to `IsoDecDeconParamsViewModel.cs`.

Expanded unit tests:
- Removed obsolete `GuiFunctionsTest.cs`.
- Added new test classes for `ClassicDeconParamsViewModel`, `DeconParamsViewModel`, `DeconHostViewModel`, and `MzLibExtensions`.
- Included tests for property changes, validation logic, and conversion methods.
Updated mzLib package version from 5.2.10 to 5.2.15 in multiple project files. Added a new property `UseProvidedPrecursors` in `SearchTaskWindow.xaml.cs`. Introduced support for `IsoDecDeconvolution` in `DeconvolutionTypeToControlConverter.cs`. Refactored `HostDeconParamControl.xaml` for simpler binding. Removed `IsoDecDeconParamsControl.xaml` and its code-behind file. Added new `IsoDecDeconParamControl.xaml` and `IsoDecDeconParamControl.xaml.cs` for detailed UI and interaction logic. Enhanced `IsoDecDeconParamsViewModel` with new properties. Added `IsoDecDeconParamsViewModelTest` for validation. Updated `SearchTaskTest.cs` to include `IsoDecDeconvolutionParameters`.
- Updated DeconHostViewModel to use AnalyteType enum.
- Removed IEquatable from DeconParamsViewModel; now uses UniqueIdentifier for hash code.
- Minor formatting change in IsoDecDeconParamsViewModel.
- Renamed and added assertions in DeconHostViewModelTests.
- Updated DeconParamsViewModelTest with new and renamed tests.
Copy link

codecov bot commented Jan 14, 2025

Codecov Report

Attention: Patch coverage is 94.30380% with 9 lines in your changes missing coverage. Please review.

Project coverage is 93.90%. Comparing base (e6cf8e7) to head (8252d05).

Files with missing lines Patch % Lines
...ons/ViewModels/Deconvolution/DeconHostViewModel.cs 89.47% 2 Missing and 2 partials ⚠️
MetaMorpheus/TaskLayer/MetaMorpheusTask.cs 86.20% 3 Missing and 1 partial ⚠️
...Models/Deconvolution/IsoDecDeconParamsViewModel.cs 98.80% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #2442    +/-   ##
========================================
  Coverage   93.89%   93.90%            
========================================
  Files         146      147     +1     
  Lines       22206    22342   +136     
  Branches     3059     3066     +7     
========================================
+ Hits        20851    20980   +129     
- Misses        906      911     +5     
- Partials      449      451     +2     
Files with missing lines Coverage Δ
MetaMorpheus/EngineLayer/CommonParameters.cs 95.67% <100.00%> (ø)
MetaMorpheus/GuiFunctions/MzLibExtensions.cs 100.00% <100.00%> (ø)
...aMorpheus/GuiFunctions/ViewModels/BaseViewModel.cs 91.30% <ø> (ø)
...s/ViewModels/Deconvolution/DeconParamsViewModel.cs 100.00% <100.00%> (+2.24%) ⬆️
...Models/Deconvolution/IsoDecDeconParamsViewModel.cs 98.82% <98.80%> (ø)
...ons/ViewModels/Deconvolution/DeconHostViewModel.cs 94.39% <89.47%> (-2.71%) ⬇️
MetaMorpheus/TaskLayer/MetaMorpheusTask.cs 89.02% <86.20%> (-0.11%) ⬇️

@nbollis nbollis changed the title [Draft for Collaborator] IsoDec incorporation IsoDec incorporation Jan 14, 2025
@nbollis nbollis marked this pull request as ready for review January 14, 2025 02:12
@trishorts
Copy link
Contributor

trishorts commented Jan 17, 2025

I'm wondering if there are different default parameters for bottom up and top down. I ran default isodec for bottom up and the results are less than classic. probably that makes sense. but i wonder if i make adjustments if isodec bottom up results can be improved. do we need explainers to help users?

top down results are similar but not the same which make sense. not sure what to think about tha.

trishorts
trishorts previously approved these changes Jan 17, 2025
@nbollis
Copy link
Member Author

nbollis commented Jan 17, 2025

I'm wondering if there are different default parameters for bottom up and top down. I ran default isodec for bottom up and the results are less than classic. probably that makes sense. but i wonder if i make adjustments if isodec bottom up results can be improved. do we need explainers to help users?

top down results are similar but not the same which make sense. not sure what to think about tha.

Default parameters for top down and bottom up are found in DeconHostViewModel. Your results are similar to what I get for top-down. I simply copied the default bottom-up parameters from classic deconvolution, so there is likely a more optimal default.

trishorts
trishorts previously approved these changes Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants