More graceful handling of libraries containing nonsensical entries (e.g. water loss on molecule containing no oxygen atoms)

pwiz_tools/Skyline/Model/AbstractModificationMatcher.cs

@@ -22,6 +22,7 @@
 using System.IO;
 using System.Linq;
 using System.Text;
+using pwiz.Common.SystemUtil;
 using pwiz.Skyline.Model.Crosslinking;
 using pwiz.Skyline.Model.DocSettings;
 using pwiz.Skyline.Model.Lib;
@@ -473,24 +474,31 @@ public PeptideDocNode CreateDocNodeFromSettings(LibKey key, Peptide peptide, Srm
                     SpectrumHeaderInfo libInfo;
                     if (nodePep != null && Settings.PeptideSettings.Libraries.TryGetLibInfo(key, out libInfo))
                     {
-                        var isotopeLabelType = key.Adduct.HasIsotopeLabels ? IsotopeLabelType.heavy : IsotopeLabelType.light;
-                        var group = new TransitionGroup(peptide, key.Adduct, isotopeLabelType);
-                        nodeGroupMatched = new TransitionGroupDocNode(group, Annotations.EMPTY, Settings, null, libInfo, ExplicitTransitionGroupValues.EMPTY, null, null, false);
-                        SpectrumPeaksInfo spectrum;
-                        if (Settings.PeptideSettings.Libraries.TryLoadSpectrum(key, out spectrum))
+                        try
                         {
-                            // Add fragment and precursor transitions as needed
-                            var transitionDocNodes =
-                                Settings.TransitionSettings.Filter.SmallMoleculeIonTypes.Contains(IonType.precursor)
-                                    ? nodeGroupMatched.GetPrecursorChoices(Settings, null, true) // Gives list of precursors
-                                    : new List<DocNode>();
-
-                            if (Settings.TransitionSettings.Filter.SmallMoleculeIonTypes.Contains(IonType.custom))
+                            var isotopeLabelType = key.Adduct.HasIsotopeLabels ? IsotopeLabelType.heavy : IsotopeLabelType.light;
+                            var group = new TransitionGroup(peptide, key.Adduct, isotopeLabelType);
+                            nodeGroupMatched = new TransitionGroupDocNode(group, Annotations.EMPTY, Settings, null, libInfo, ExplicitTransitionGroupValues.EMPTY, null, null, false);
+                            SpectrumPeaksInfo spectrum;
+                            if (Settings.PeptideSettings.Libraries.TryLoadSpectrum(key, out spectrum))
                             {
-                                GetSmallMoleculeFragments(key, nodeGroupMatched, spectrum, transitionDocNodes);
+                                // Add fragment and precursor transitions as needed
+                                var transitionDocNodes =
+                                    Settings.TransitionSettings.Filter.SmallMoleculeIonTypes.Contains(IonType.precursor)
+                                        ? nodeGroupMatched.GetPrecursorChoices(Settings, null, true) // Gives list of precursors
+                                        : new List<DocNode>();
+
+                                if (Settings.TransitionSettings.Filter.SmallMoleculeIonTypes.Contains(IonType.custom))
+                                {
+                                    GetSmallMoleculeFragments(key, nodeGroupMatched, spectrum, transitionDocNodes);
+                                }
+                                nodeGroupMatched = (TransitionGroupDocNode)nodeGroupMatched.ChangeChildren(transitionDocNodes);
+                                return (PeptideDocNode)nodePep.ChangeChildren(new List<DocNode>() { nodeGroupMatched });
                             }
-                            nodeGroupMatched = (TransitionGroupDocNode)nodeGroupMatched.ChangeChildren(transitionDocNodes);
-                            return (PeptideDocNode)nodePep.ChangeChildren(new List<DocNode>() { nodeGroupMatched });
+                        }
+                        catch (InvalidChemicalModificationException e)
+                        {
+                            Messages.WriteAsyncUserMessage(e.Message); // Adduct makes no sense for target formula
                         }
                     }
                 }

pwiz_tools/Skyline/Model/Import.cs

@@ -836,7 +836,7 @@ private PeptideGroupBuilder AddRow(PeptideGroupBuilder seqBuilder,
             {
                 seqBuilder.AppendTransition(info, irt, explicitRT, libraryIntensity, productMz, note, lineText, lineNum);
             }
-            catch (InvalidDataException x)
+            catch (Exception x) when (!ExceptionUtil.IsProgrammingDefect(x))
             {
                 throw new LineColNumberedIoException(x.Message, lineNum, -1, x);
             }

pwiz_tools/Skyline/Model/Lib/SpectrumRanker.cs

@@ -173,11 +173,19 @@ public SpectrumRanker(TargetInfo targetInfo, SrmSettings settings,
                     var ionMasses =
                         new IonMasses(calcMatch.GetPrecursorFragmentMass(Sequence), IonTable<TypedMass>.EMPTY)
                             .ChangeKnownFragments(knownFragments);
-                    moleculeMasses =
-                        new MoleculeMasses(
-                            SequenceMassCalc.GetMZ(
-                                calcMatchPre.GetPrecursorMass(Sequence.Molecule, null, PrecursorAdduct,
-                                    out _), PrecursorAdduct), ionMasses);
+                    try
+                    {
+                        moleculeMasses =
+                            new MoleculeMasses(
+                                SequenceMassCalc.GetMZ(
+                                    calcMatchPre.GetPrecursorMass(Sequence.Molecule, null, PrecursorAdduct,
+                                        out _), PrecursorAdduct), ionMasses);
+                    }
+                    catch (InvalidChemicalModificationException)
+                    {
+                        moleculeMasses =
+                            new MoleculeMasses(double.NaN, ionMasses); // Precursor m/z can't be calculated
+                    }
                 }
                 else
                 {

pwiz_tools/Skyline/Model/Lib/ViewLibraryPepInfo.cs

@@ -236,8 +236,19 @@ public double CalcMz(SrmSettings settings,
         {
             TypedMass massH;
             if (Target != null)
-                massH = settings.GetPrecursorCalc(transitionGroup.TransitionGroup.LabelType, mods)
-                    .GetPrecursorMass(Target);
+            {
+                if (!Target.IsProteomic)
+                {
+                    massH = SequenceMassCalc.FormulaMass(BioMassCalc.MONOISOTOPIC,
+                        transitionGroup.PrecursorAdduct.ApplyToMolecule(Target.Molecule.ParsedMolecule));
+                    return SequenceMassCalc.PersistentMZ(SequenceMassCalc.GetMZ(massH, transitionGroup.PrecursorAdduct.AdductCharge));
+                }
+                else
+                {
+                    massH = settings.GetPrecursorCalc(transitionGroup.TransitionGroup.LabelType, mods)
+                        .GetPrecursorMass(Target);
+                }
+            }
             else
                 massH = new TypedMass(Key.PrecursorMz ?? 0, MassType.Monoisotopic);
             return SequenceMassCalc.PersistentMZ(SequenceMassCalc.GetMZ(massH, transitionGroup.PrecursorAdduct));

pwiz_tools/Skyline/Model/SmallMoleculeTransitionListReader.cs

@@ -382,6 +382,7 @@ public static bool IsParserException(Exception exception)
         {
             return exception is InvalidOperationException
                    || exception is InvalidDataException
+                   || exception is InvalidChemicalModificationException
                    || exception is ArgumentException;
         }
 

pwiz_tools/Skyline/SettingsUI/ViewLibraryDlg.cs

@@ -921,6 +921,11 @@ public void UpdateUI(bool selectionChanged = true)
                 SetGraphItem(new NoDataMSGraphItem(SettingsUIResources.ViewLibraryDlg_UpdateUI_Unauthorized_access_attempting_to_read_from_library_));
                 return;
             }
+            catch (InvalidChemicalModificationException e)
+            {
+                SetGraphItem(new NoDataMSGraphItem(e.Message));
+                return;
+            }
             catch (IOException)
             {
                 SetGraphItem(new NoDataMSGraphItem(SettingsUIResources.ViewLibraryDlg_UpdateUI_Failure_loading_spectrum_Library_may_be_corrupted));
@@ -2674,15 +2679,25 @@ public PeptideTipProvider(ViewLibraryPepInfo pepInfo, LibKeyModificationMatcher
                     _smallMoleculePartsToDraw = smallMolInfo.LocalizedKeyValuePairs;
                 }
 
+                _mzRangePartsToDraw = new List<TextColor>();
                 if (_pepInfo.Target != null)
                 {
                     // build mz range parts to draw
-                    _mz = _pepInfo.CalcMz(_settings, transitionGroup, mods);
-                    _mzRangePartsToDraw = GetMzRangeItemsToDraw(_mz);
+                    try
+                    {
+                        _mz = _pepInfo.CalcMz(_settings, transitionGroup, mods);
+                        _mzRangePartsToDraw = GetMzRangeItemsToDraw(_mz);
+                    }
+                    catch (InvalidChemicalModificationException e)
+                    {
+                        _mz = double.NaN;
+                        // Show the error at the top of the tip
+                        _seqPartsToDraw.Insert(0,new TextColor(e.Message, Brushes.Red));
+                        _seqPartsToDraw.Insert(1, new TextColor(Environment.NewLine));
+                    }
                 }
                 else
                 {
-                    _mzRangePartsToDraw = new List<TextColor>();
                     var precursorKey = _pepInfo.Key.LibraryKey as PrecursorLibraryKey;
                     if (precursorKey != null)
                     {
@@ -2925,13 +2940,23 @@ private SizeF DrawTextParts(Graphics g, float startX, float startY, List<TextCol
             {
                 var size = new SizeF(startX, startY);
                 float height = 0;
+                float lineWidth = 0;
+                float width = 0;
+                float nLines = 1;
                 foreach (var part in parts)
                 {
-                    g.DrawString(part.Text, rt.FontNormal, part.Color, new PointF(size.Width, size.Height));
-                    size.Width += g.MeasureString(part.Text, rt.FontNormal).Width - 3;
-                    height = g.MeasureString(part.Text, rt.FontNormal).Height;
+                    if (part.Text.StartsWith(Environment.NewLine))
+                    {
+                        lineWidth = 0;
+                        nLines++;
+                    }
+                    g.DrawString(part.Text, rt.FontNormal, part.Color, new PointF(startX + lineWidth, startY + height));
+                    lineWidth += g.MeasureString(part.Text, rt.FontNormal).Width - 3;
+                    width = Math.Max(lineWidth, width);
+                    height = Math.Max(height, g.MeasureString(part.Text, rt.FontNormal).Height);
                 }
-                size.Height = height;
+                size.Height = nLines * height;
+                size.Width = width;
                 return size;
             }
 
@@ -3016,11 +3041,14 @@ public void UpdateRedundantComboItems()
             if (!_comboBoxUpdated)
             {
                 comboRedundantSpectra.BeginUpdate();
-                foreach (ComboOption opt in _currentOptions)
+                if (_currentOptions != null)
                 {
-                    if (!opt.SpectrumInfoLibrary.IsBest)
+                    foreach (ComboOption opt in _currentOptions)
                     {
-                        comboRedundantSpectra.Items.Add(opt);
+                        if (!opt.SpectrumInfoLibrary.IsBest)
+                        {
+                            comboRedundantSpectra.Items.Add(opt);
+                        }
                     }
                 }
 

pwiz_tools/Skyline/Test/AdductTest.cs

@@ -99,7 +99,7 @@ private void TestTaxolAdduct(string adductText, double expectedMz, int expectedC
             coverage.Add(adduct.AsFormula());
         }
 
-        private void TestException(string formula, string adductText)
+        private void TestInvalidDataException(string formula, string adductText)
         {
             AssertEx.ThrowsException<InvalidDataException>(() =>
             {
@@ -108,6 +108,15 @@ private void TestException(string formula, string adductText)
             });
         }
 
+        private void TestInvalidChemicalModificationException(string formula, string adductText)
+        {
+            AssertEx.ThrowsException<InvalidChemicalModificationException>(() =>
+            {
+                var adduct = Adduct.FromStringAssumeProtonated(adductText);
+                IonInfo.ApplyAdductToFormula(formula, adduct);
+            });
+        }
+
         private string RoundtripFormulaString(string f)
         {
             ParsedMolecule.TryParseFormula(f, out var mol, out _);
@@ -427,19 +436,19 @@ public void AdductParserTest()
             mz = BioMassCalc.CalculateIonMass(new TypedMass(massHectochlorin, MassType.Monoisotopic), heavy);
             Assert.AreEqual(2 * (massHectochlorin + 1.23456), mz, .001);
 
-            TestException(PENTANE, "zM+2H"); // That "z" doesn't make any sense as a mass multiplier (must be a positive integer)
-            TestException(PENTANE, "-2M+2H"); // That "-2" doesn't make any sense as a mass multiplier (must be a positive integer)
-            TestException("", "+M"); // Meaningless, used to cause an exception in our parser
-            TestException(Hectochlorin, "M3Cl37+H"); // Trying to label more chlorines than exist in the molecule
-            TestException(Hectochlorin, "M-3Cl+H"); // Trying to remove more chlorines than exist in the molecule
-            TestException(PENTANE, "M+foo+H"); // Unknown adduct
-            TestException(PENTANE, "M2Cl37H+H"); // nonsense label ("2Cl37H2" would make sense, but regular H doesn't belong)
-            TestException(PENTANE, "M+2H+"); // Trailing sign - we now understand this as a charge state declaration, but this one doesn't match described charge
-            TestException(PENTANE, "[M-2H]3-"); // Declared charge doesn't match described charge
-            TestException(PENTANE, "[M-]3-"); // Declared charge doesn't match described charge
-            TestException(PENTANE, "[M+]-"); // Declared charge doesn't match described charge
-            TestException(PENTANE, "[M+2]-"); // Declared charge doesn't match described charge
-            TestException(PENTANE, "[M+2]+3"); // Declared charge doesn't match described charge
+            TestInvalidDataException(PENTANE, "zM+2H"); // That "z" doesn't make any sense as a mass multiplier (must be a positive integer)
+            TestInvalidDataException(PENTANE, "-2M+2H"); // That "-2" doesn't make any sense as a mass multiplier (must be a positive integer)
+            TestInvalidDataException("", "+M"); // Meaningless, used to cause an exception in our parser
+            TestInvalidChemicalModificationException(Hectochlorin, "M3Cl37+H"); // Trying to label more chlorines than exist in the molecule
+            TestInvalidChemicalModificationException(Hectochlorin, "M-3Cl+H"); // Trying to remove more chlorines than exist in the molecule
+            TestInvalidDataException(PENTANE, "M+foo+H"); // Unknown adduct
+            TestInvalidDataException(PENTANE, "M2Cl37H+H"); // nonsense label ("2Cl37H2" would make sense, but regular H doesn't belong)
+            TestInvalidDataException(PENTANE, "M+2H+"); // Trailing sign - we now understand this as a charge state declaration, but this one doesn't match described charge
+            TestInvalidDataException(PENTANE, "[M-2H]3-"); // Declared charge doesn't match described charge
+            TestInvalidDataException(PENTANE, "[M-]3-"); // Declared charge doesn't match described charge
+            TestInvalidDataException(PENTANE, "[M+]-"); // Declared charge doesn't match described charge
+            TestInvalidDataException(PENTANE, "[M+2]-"); // Declared charge doesn't match described charge
+            TestInvalidDataException(PENTANE, "[M+2]+3"); // Declared charge doesn't match described charge
 
             // Test label stripping
             Assert.AreEqual("C5H9NO2S", (new IonInfo("C5H9H'3NO2S[M-3H]")).UnlabeledFormula.ToString());

pwiz_tools/Skyline/TestFunctional/LibraryBuildTest.cs

@@ -175,6 +175,20 @@ private void MainTest()
             RunUI(() => AssertEx.IsTrue(viewLibUI.GraphItem.IonLabels.Any()));
             OkDialog(viewLibUI, viewLibUI.CancelDialog);
 
+            // Now check for handling when adduct tries to label more atoms than are present in the molecule
+            var sslLines = File.ReadAllLines(TestFilesDir.GetTestPath("library_valid\\heavy_adduct.ssl")).ToList();
+            var badSSL = sslLines[1].Replace("ms2\t2","ms2\t3").Replace(@"M6C13", @"M66C13"); // Molecule is C54H83N15O20 so this makes no sense
+            sslLines.Add(badSSL);
+            File.WriteAllLines(TestFilesDir.GetTestPath("library_valid\\heavy_adduct_bad.ssl"), sslLines);
+            BuildLibraryValid("heavy_adduct_bad.ssl", true, false, false, 2);
+            // Make sure explorer handles this adduct, which is a bad match for the molecule
+            viewLibUI = ShowDialog<ViewLibraryDlg>(SkylineWindow.ViewSpectralLibraries);
+            RunUI(() => AssertEx.IsTrue(viewLibUI.GraphItem.IonLabels.Any()));
+            // Add All should cause some notifications since one of them is bad
+            var filterMatchedDlg = ShowDialog<FilterMatchedPeptidesDlg>(() => viewLibUI.AddAllPeptides());
+            OkDialog(filterMatchedDlg, filterMatchedDlg.CancelDialog);
+            OkDialog(viewLibUI, viewLibUI.CancelDialog);
+
             // Barbara added code to ProteoWizard to rebuild a missing or invalid mzXML index
             // BuildLibraryError("bad_mzxml.pep.XML", "<index> not found");
             BuildLibraryValid(TestFilesDir.GetTestPath("library_errors"), new[] { "bad_mzxml.pep.XML" }, false, false, false, 1, 0, false);

pwiz_tools/Skyline/Util/Adduct.cs

@@ -66,6 +66,17 @@ public T this[Adduct a]
         public IEnumerable<Adduct> Keys { get { return _dict.Keys; } }
     }
 
+    /// <summary>
+    /// Thrown when a loss would remove more atoms than are found in a molecular formula e.g. water loss from C2S5H12 (no oxygen),
+    /// or label would change more atoms than are in the formula e.g. 3C' on C2S5H12
+    /// </summary>
+    public class InvalidChemicalModificationException : IOException
+    {
+        public InvalidChemicalModificationException(string message) : base(message)
+        {
+        }
+    }
+
     public class Adduct : Immutable, IComparable, IEquatable<Adduct>, IAuditLogObject
     {
         private Molecule Composition { get; set; } // The chemical makeup of the adduct - the "2H" part in 4M3Cl37+2H
@@ -1400,7 +1411,7 @@ public ParsedMolecule ApplyToMolecule(ParsedMolecule molecule)
                 if (!molecule.IsMassOnly &&
                     count < 0 && !Equals(pair.Key, BioMassCalc.H)) // Treat H loss as a general proton loss
                 {
-                    throw new InvalidDataException(
+                    throw new InvalidChemicalModificationException(
                         string.Format(Resources.Adduct_ApplyToMolecule_Adduct___0___calls_for_removing_more__1__atoms_than_are_found_in_the_molecule__2_,
                             this, pair.Key, molecule.ToString()));
                 }
@@ -1460,7 +1471,7 @@ private ParsedMolecule ApplyIsotopeValues(ParsedMolecule molecule, int massMulti
                     }
                     else // Can't remove that which is not there
                     {
-                        throw new InvalidDataException(
+                        throw new InvalidChemicalModificationException(
                             string.Format(
                                 UtilResources
                                     .Adduct_ApplyToMolecule_Adduct___0___calls_for_labeling_more__1__atoms_than_are_found_in_the_molecule__2_,