changed alignment mass tolearnce to 0.5 Da#710
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to increase the mass tolerance used during alignment (notably to 0.5 Da), presumably to better fit nominal-mass GCMS alignment behavior.
Changes:
- Replaced the
GcmsPeakJoinerconstructor’s use of the providedMsRefSearchParameterBasewith a newly constructed parameter fixed at 0.5 tolerances and a 0–2000 mass range. - Updated the default
AlignmentBaseParameter.Ms1AlignmentTolerancefrom0.015Fto0.5Fin the shared parameter model.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/MSDIAL5/MsdialGcMsApi/Algorithm/Alignment/GcmsPeakJoiner.cs |
Forces a fixed MsRefSearchParameterBase for GCMS alignment comparisons, overriding caller-provided search parameters. |
src/MSDIAL5/MsdialCore/Parameter/ParameterBase.cs |
Raises the default MS1 alignment tolerance globally via AlignmentBaseParameter. |
Comments suppressed due to low confidence (3)
src/MSDIAL5/MsdialGcMsApi/Algorithm/Alignment/GcmsPeakJoiner.cs:48
- Hard-coded matching tolerances and mass range (
0.5F,0..2000) here bypass user/project settings (e.g.,GcmsParameter.MspSearchParamand/or peak-pick mass range). Consider deriving these values from the passed search parameter and/orparameter.PeakPickBaseParamso alignment behavior stays consistent with the rest of the pipeline.
_msMatchParam = new MsRefSearchParameterBase() { Ms1Tolerance = 0.5F, Ms2Tolerance = 0.5F, MassRangeBegin = 0, MassRangeEnd = 2000 };
src/MSDIAL5/MsdialCore/Parameter/ParameterBase.cs:1151
- Changing the default
Ms1AlignmentTolerancefrom0.015Fto0.5FinAlignmentBaseParameteraffects all MSDIAL5 workflows (LCMS/IMMS/DIMS/GCMS), not just GCMS alignment. If the intent is GCMS-specific nominal-mass behavior, consider setting this default inMsdialGcmsParameterinitialization (or UI preset logic) instead of the shared base parameter to avoid widening tolerances for high-resolution modes by default.
public float Ms1AlignmentTolerance { get; set; } = 0.5F;
src/MSDIAL5/MsdialGcMsApi/Algorithm/Alignment/GcmsPeakJoiner.cs:48
- This change alters GCMS alignment matching behavior by overriding the search parameter at construction time, but there doesn't appear to be unit coverage validating that
GcmsPeakJoinerrespects configuredMsRefSearchParameterBasesettings. Adding a test intests/MSDIAL5/MsdialGcMsApiTeststhat exercisesGcmsAlignmentProcessFactory.CreatePeakJoiner()and asserts the joiner uses the provided search parameter (e.g., Ms1Tolerance/MassRange) would help prevent regressions.
_msMatchParam = new MsRefSearchParameterBase() { Ms1Tolerance = 0.5F, Ms2Tolerance = 0.5F, MassRangeBegin = 0, MassRangeEnd = 2000 };
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _alignmentParameter = parameter.AlignmentBaseParam; | ||
| _riCompoundType = riCompoundType; | ||
| _msMatchParam = msMatchParam; | ||
| _msMatchParam = new MsRefSearchParameterBase() { Ms1Tolerance = 0.5F, Ms2Tolerance = 0.5F, MassRangeBegin = 0, MassRangeEnd = 2000 }; |
There was a problem hiding this comment.
The constructor ignores the provided msMatchParam argument and always replaces it with a new MsRefSearchParameterBase. This makes CreateRTJoiner/CreateRIJoiner's msMatchParam parameter ineffective and prevents callers from configuring spectrum matching behavior. Use the passed-in parameter (or a copy of it) and only override specific fields if needed.
| _msMatchParam = new MsRefSearchParameterBase() { Ms1Tolerance = 0.5F, Ms2Tolerance = 0.5F, MassRangeBegin = 0, MassRangeEnd = 2000 }; | |
| _msMatchParam = msMatchParam ?? new MsRefSearchParameterBase() { Ms1Tolerance = 0.5F, Ms2Tolerance = 0.5F, MassRangeBegin = 0, MassRangeEnd = 2000 }; |
No description provided.