Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions pwiz_tools/Skyline/Model/Results/ResultsResources.designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions pwiz_tools/Skyline/Model/Results/ResultsResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,15 @@ It is recommended that you delete this file so that Skyline can create a new fil
<data name="SpectrumFilter_CalcDiaIsolationValues_Unable_to_determine_isolation_width_for_the_scan_targeted_at__0_" xml:space="preserve">
<value>Unable to determine isolation width for the scan targeted at {0}</value>
</data>
<data name="SpectrumFilter_IonMobilityFilteringDisabled_EnableSpectralLibrary" xml:space="preserve">
<value>Note: the spectral library has ion mobility values for "{0}", but they are being ignored because "Use spectral library ion mobility values when present" is not enabled. To apply ion mobility filtering, open Settings &gt; Transition Settings &gt; Ion Mobility and enable that option.</value>
</data>
<data name="SpectrumFilter_IonMobilityFilteringDisabled_SetWindow" xml:space="preserve">
<value>Note: ion mobility values are available for "{0}", but no effective filter window is configured. To apply ion mobility filtering, open Settings &gt; Transition Settings &gt; Ion Mobility and set a window type (resolving power, fixed width, or linear range) with a non-zero value.</value>
</data>
<data name="SpectrumFilter_IonMobilityFilteringDisabled_SetWindowAndEnableSpectralLibrary" xml:space="preserve">
<value>Note: the spectral library has ion mobility values for "{0}", but ion mobility filtering is disabled. To apply filtering, open Settings &gt; Transition Settings &gt; Ion Mobility, set a window type (resolving power, fixed width, or linear range) with a value, and enable "Use spectral library ion mobility values when present".</value>
</data>
<data name="TransitionBinner_BinData_" xml:space="preserve">
<value>Imported spectrum appears to be missing m/z or intensity values ({0} != {1})</value>
</data>
Expand Down
150 changes: 134 additions & 16 deletions pwiz_tools/Skyline/Model/Results/SpectrumFilter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -141,23 +141,19 @@ public SpectrumFilter(SrmDocument document, MsDataFileUri msDataFileUri, IFilter
// TIC and Base peak are meaningless with FAIMS, where we can't know the actual overall ion counts -also can't reliably share times with any ion mobility scheme
if (instrumentInfo != null && instrumentInfo.IonMobilityUnits != eIonMobilityUnits.none)
{
if ((libraryIonMobilityInfo != null && !libraryIonMobilityInfo.IsEmpty) || _isWatersSonar)
_isIonMobilityFiltered = WouldApplyIonMobilityFiltering(
document.Settings, libraryIonMobilityInfo, moleculesThisPass, ionMobilityMax);

// Issue 4150: if filtering would not apply under current settings, probe simple
// settings variations to see whether a specific setting change would enable it,
// and warn the user about the specific change. When a retention time predictor
// is in use, SpectrumFilter is constructed twice for the same file (first pass
// sees only first-pass peptides, second pass sees all molecules) - skip the
// first pass so the warning fires only once and with the complete molecule set.
if (!_isIonMobilityFiltered && (retentionTimePredictor == null || !firstPass))
{
_isIonMobilityFiltered = true;
}
else
{
foreach (var pair in moleculesThisPass.SelectMany(
node => node.TransitionGroups.Select(nodeGroup => new PeptidePrecursorPair(node, nodeGroup))))
{
var ionMobility = document.Settings.GetIonMobilityFilter(
pair.NodePep, pair.NodeGroup, null, libraryIonMobilityInfo, _ionMobilityFunctionsProvider, ionMobilityMax);
_isIonMobilityFiltered = ionMobility.HasIonMobilityValue;
if (_isIonMobilityFiltered)
{
break;
}
}
WarnIfSettingsChangeWouldEnableFiltering(
document.Settings, libraryIonMobilityInfo, moleculesThisPass, msDataFileUri, ionMobilityMax);
}
Comment thread
bspratt marked this conversation as resolved.
}

Expand Down Expand Up @@ -490,6 +486,128 @@ public SpectrumFilter(SrmDocument document, MsDataFileUri msDataFileUri, IFilter
InitIonMobilityAndRTLimits();
}

// Representative window used solely to probe "would enabling a filter window turn
// filtering on for this data?" - value is arbitrary, just needs to be non-empty.
private const double PROBE_RESOLVING_POWER = 50;

/// <summary>
/// Returns true if ion mobility filtering would actually produce a filter for at least
/// one precursor in the document under the given settings. Used both to set
/// <see cref="_isIonMobilityFiltered"/> from real settings and to probe hypothetical
/// settings variations when diagnosing a "filtering disabled" warning (issue 4150).
/// </summary>
private bool WouldApplyIonMobilityFiltering(SrmSettings settings,
LibraryIonMobilityInfo libraryIonMobilityInfo,
IList<PeptideDocNode> molecules, double ionMobilityMax)
{
// Waters SONAR maps mz bins to IM; the "filter" is always derived from the mz filter.
if (_isWatersSonar)
{
return true;
}

foreach (var pair in molecules.SelectMany(
node => node.TransitionGroups.Select(nodeGroup => new PeptidePrecursorPair(node, nodeGroup))))
{
var ionMobility = settings.GetIonMobilityFilter(
pair.NodePep, pair.NodeGroup, null, libraryIonMobilityInfo, _ionMobilityFunctionsProvider, ionMobilityMax);
if (ionMobility != null && ionMobility.HasIonMobilityValue)
{
return true;
}
}
return false;
}

// Issue 4150: with current settings no precursor produces an ion mobility filter. Probe
// the obvious setting variations (add a window width, enable spectral-library IM, both)
// to see whether a specific change would enable filtering. If one would, emit a
// non-blocking notice that tells the user exactly which setting to flip. Spectral-library
// IM info is settings-invariant so can be reused across probes.
private void WarnIfSettingsChangeWouldEnableFiltering(SrmSettings settings,
LibraryIonMobilityInfo libraryIonMobilityInfo,
IList<PeptideDocNode> molecules, MsDataFileUri msDataFileUri, double ionMobilityMax)
{
bool WouldWith(SrmSettings probe) =>
WouldApplyIonMobilityFiltering(probe, libraryIonMobilityInfo, molecules, ionMobilityMax);

// Skip probes that can't change the outcome: if a setting is already on, the
// corresponding "With..." variation is a no-op, and since base filtering is already
// false that probe would also be false. When both are on, no setting change can
// enable filtering, so there's no actionable warning.
var imFiltering = settings.TransitionSettings.IonMobilityFiltering;
var existingCalc = imFiltering?.FilterWindowWidthCalculator;
bool hasWindow = HasUsableWindow(existingCalc);
bool hasSpectralLib = imFiltering?.UseSpectralLibraryIonMobilityValues ?? false;
if (hasWindow && hasSpectralLib)
{
return;
}

// Prefer the user's existing window if one is both selected and usable - probing with
// their real values keeps the "would this change fix it?" test faithful instead of
// substituting an arbitrary resolving power. Fall back to a default probe if no
// window type is set OR the selected type has a zero-valued primary field
// (e.g., fixed_width=0, linear_range widths both 0), which would produce a
// zero-width filter and make the probe a silent no-op.
var probeWindow = hasWindow
? existingCalc
: new IonMobilityWindowWidthCalculator(PROBE_RESOLVING_POWER);
SrmSettings WithWindow(SrmSettings s) => s.ChangeTransitionSettings(
s.TransitionSettings.ChangeIonMobilityFiltering(
s.TransitionSettings.IonMobilityFiltering.ChangeFilterWindowWidthCalculator(probeWindow)));
SrmSettings WithSpectralLib(SrmSettings s) => s.ChangeTransitionSettings(
s.TransitionSettings.ChangeIonMobilityFiltering(
s.TransitionSettings.IonMobilityFiltering.ChangeUseSpectralLibraryIonMobilityValues(true)));

string message = null;
if (!hasWindow && WouldWith(WithWindow(settings)))
{
// Window alone is enough: IMSDB or explicit IM is present.
message = ResultsResources.SpectrumFilter_IonMobilityFilteringDisabled_SetWindow;
}
else if (!hasSpectralLib && WouldWith(WithSpectralLib(settings)))
{
// Enabling spectral-lib IM alone is enough: window is already configured.
message = ResultsResources.SpectrumFilter_IonMobilityFilteringDisabled_EnableSpectralLibrary;
}
else if (!hasWindow && !hasSpectralLib && WouldWith(WithSpectralLib(WithWindow(settings))))
{
Comment on lines +531 to +575
// Need both: no IMSDB or explicit IM, but spectral-lib IM is usable once enabled and a window is set.
message = ResultsResources.SpectrumFilter_IonMobilityFilteringDisabled_SetWindowAndEnableSpectralLibrary;
}
// else: no setting change would enable filtering, so there's no actionable warning.

if (message != null)
{
var fileName = msDataFileUri?.GetFileName() ?? string.Empty;
Messages.WriteAsyncUserMessage(message, fileName);
}
}

// A calculator is "usable" as a probe only if its active mode has a non-zero primary
// field. A mode-selected-but-all-zero calculator produces a zero-width filter that makes
// the WithWindow probe a silent no-op, which would incorrectly suppress the warning.
private static bool HasUsableWindow(IonMobilityWindowWidthCalculator calc)
{
if (calc == null)
{
return false;
}
switch (calc.WindowWidthMode)
{
case IonMobilityWindowWidthCalculator.IonMobilityWindowWidthType.resolving_power:
return calc.ResolvingPower > 0;
case IonMobilityWindowWidthCalculator.IonMobilityWindowWidthType.fixed_width:
return calc.FixedWindowWidth > 0;
case IonMobilityWindowWidthCalculator.IonMobilityWindowWidthType.linear_range:
return calc.PeakWidthAtIonMobilityValueZero > 0 ||
calc.PeakWidthAtIonMobilityValueMax > 0;
default:
return false;
}
}

public bool ProvidesCollisionalCrossSectionConverter { get { return _ionMobilityFunctionsProvider != null; } }

public eIonMobilityUnits IonMobilityUnits
Expand Down
36 changes: 36 additions & 0 deletions pwiz_tools/Skyline/TestFunctional/CrosslinkImsTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@
*/
using System.Linq;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using pwiz.Skyline.Controls;
using pwiz.Skyline.Model;
using pwiz.Skyline.Model.DocSettings;
using pwiz.Skyline.Model.Lib;
using pwiz.Skyline.Model.Results;
using pwiz.Skyline.SettingsUI;
using pwiz.Skyline.SettingsUI.IonMobility;
using pwiz.SkylineTestUtil;
Expand Down Expand Up @@ -98,6 +101,39 @@ protected override void DoTest()
ionMobilityLibraryDlg.OkDialog();
});
OkDialog(transitionSettingsUi, transitionSettingsUi.OkDialog);

VerifyIonMobilityFilteringDisabledWarning();
}

// Issue 4150: when re-importing data while the IM filter window is disabled but IM values
// remain available (here from the IMSDB created earlier in this test), Skyline should
// emit a non-blocking informational message to the Immediate Window naming the specific
// fix - in this case, setting a filter window width.
private void VerifyIonMobilityFilteringDisabledWarning()
{
const string fileName = "CrosslinkImsTest.mzML";

RunUI(() => SkylineWindow.ModifyDocument("Disable IM filtering window", doc =>
doc.ChangeSettings(doc.Settings.ChangeTransitionSettings(
doc.Settings.TransitionSettings.ChangeIonMobilityFiltering(
doc.Settings.TransitionSettings.IonMobilityFiltering.ChangeFilterWindowWidthCalculator(
IonMobilityWindowWidthCalculator.EMPTY))))));
RunUI(() => SkylineWindow.ModifyDocument("Remove results",
doc => doc.ChangeMeasuredResults(null)));
// Save so the existing .skyd is rewritten and re-import doesn't trigger a confirmation dialog
RunUI(() => SkylineWindow.SaveDocument());

ImportResultsFile(TestFilesDir.GetTestPath(fileName));

TryWaitForOpenForm<ImmediateWindow>();
var immediateWindow = FindOpenForm<ImmediateWindow>();
Assert.IsNotNull(immediateWindow, @"Immediate window should be shown");

// The IMSDB is populated at this point, so setting a filter window alone would enable
// filtering. The diagnostic should pick the SetWindow-specific message, not a generic one.
var expected = string.Format(
ResultsResources.SpectrumFilter_IonMobilityFilteringDisabled_SetWindow, fileName);
RunUI(() => StringAssert.Contains(immediateWindow.TextContent, expected));
}
}
}
Loading