Fix Assume failure in "SpectraChromDataProvider.ChromIds" (#2787)

Fix error extracting chromatograms from raw file when there are pressure traces but no MS1 spectra (reported by Bill)

pwiz_tools/Skyline/Model/Results/ChromDataProvider.cs

@@ -21,10 +21,12 @@
 using System.Drawing;
 using System.Linq;
 using pwiz.Common.Chemistry;
+using pwiz.Common.Collections;
 using pwiz.Common.SystemUtil;
 using pwiz.ProteowizardWrapper;
 using pwiz.Skyline.Model.DocSettings;
 using pwiz.Skyline.Model.Results.Spectra;
+using pwiz.Skyline.Util;
 
 namespace pwiz.Skyline.Model.Results
 {
@@ -112,52 +114,38 @@ public virtual void SetRequestOrder(IList<IList<int>> orderedSets) { }
         public abstract void Dispose();
     }
 
+
+    /// <summary>
+    /// Keeps track of the Total Ion Current chromatogram, the base peak chromatogram, and the QC Trace chromatograms.
+    /// When this class is used by ChromDataProvider, the chromatograms are identified by the chromatogram index
+    /// in the data file.
+    /// When this class is used by SpectraChromDataProvider, the chromatograms are identified by an integer which is
+    /// an index into the list returned by <see cref="ListChromKeys"/>.
+    /// </summary>
     public sealed class GlobalChromatogramExtractor
     {
         private MsDataFileImpl _dataFile;
         private const string TIC_CHROMATOGRAM_ID = @"TIC";
         private const string BPC_CHROMATOGRAM_ID = @"BPC";
+        private Dictionary<int, MsDataFileImpl.QcTrace> _qcTracesByChromatogramIndex;
+
 
         public GlobalChromatogramExtractor(MsDataFileImpl dataFile)
         {
             _dataFile = dataFile;
-            IndexOffset = 0;
 
             if (dataFile.ChromatogramCount > 0 && dataFile.GetChromatogramId(0, out _) == TIC_CHROMATOGRAM_ID)
                 TicChromatogramIndex = 0;
             if (dataFile.ChromatogramCount > 1 && dataFile.GetChromatogramId(1, out _) == BPC_CHROMATOGRAM_ID)
                 BpcChromatogramIndex = 1;
-
-            QcTraceByIndex = new SortedDictionary<int, MsDataFileImpl.QcTrace>();
-            foreach (var qcTrace in dataFile.GetQcTraces() ?? new List<MsDataFileImpl.QcTrace>())
-            {
-                QcTraceByIndex[qcTrace.Index] = qcTrace;
-            }
+            QcTraces = ImmutableList.ValueOfOrEmpty(dataFile.GetQcTraces());
+            _qcTracesByChromatogramIndex = QcTraces.ToDictionary(qcTrace => qcTrace.Index);
         }
 
-        /// <summary>
-        /// Since global chromatograms can share the same index as Skyline's target chromatograms (they both start at 0),
-        /// the global chromatograms are given an index offset starting after the last target chromatogram
-        /// </summary>
-        public int IndexOffset { get; set; }
-
         public int? TicChromatogramIndex { get; set; }
         public int? BpcChromatogramIndex { get; }
 
-        public IList<int> GlobalChromatogramIndexes
-        {
-            get
-            {
-                var result = new List<int>();
-                if (TicChromatogramIndex.HasValue)
-                    result.Add(TicChromatogramIndex.Value);
-                if (BpcChromatogramIndex.HasValue)
-                    result.Add(BpcChromatogramIndex.Value);
-                return result;
-            }
-        }
-
-        public IDictionary<int, MsDataFileImpl.QcTrace> QcTraceByIndex { get; }
+        public ImmutableList<MsDataFileImpl.QcTrace> QcTraces { get; }
 
         public string GetChromatogramId(int index, out int indexId)
         {
@@ -166,22 +154,24 @@ public string GetChromatogramId(int index, out int indexId)
 
         public bool GetChromatogram(int index, out float[] times, out float[] intensities)
         {
-            index -= IndexOffset;
+            if (index == TicChromatogramIndex || index == BpcChromatogramIndex)
+            {
+                return ReadChromatogramFromDataFile(index, out times, out intensities);
+            }
 
-            if (QcTraceByIndex.TryGetValue(index, out MsDataFileImpl.QcTrace qcTrace))
+            if (_qcTracesByChromatogramIndex.TryGetValue(index, out var qcTrace))
             {
                 times = MsDataFileImpl.ToFloatArray(qcTrace.Times);
                 intensities = MsDataFileImpl.ToFloatArray(qcTrace.Intensities);
-            }
-            else if (index == TicChromatogramIndex || index == BpcChromatogramIndex)
-            {
-                _dataFile.GetChromatogram(index, out _, out times, out intensities, true);
-            }
-            else
-            {
-                times = intensities = null;
+                return true;
             }
 
+            times = intensities = null;
+            return false;
+        }
+        private bool ReadChromatogramFromDataFile(int chromatogramIndex, out float[] times, out float[] intensities)
+        {
+            _dataFile.GetChromatogram(chromatogramIndex, out _, out times, out intensities, true);
             return times != null;
         }
 
@@ -209,6 +199,89 @@ public bool IsTicChromatogramUsable()
 
             return true;
         }
+
+        public int ChromatogramCount
+        {
+            get
+            {
+                int count = QcTraces.Count;
+                if (TicChromatogramIndex.HasValue)
+                {
+                    count++;
+                }
+
+                if (BpcChromatogramIndex.HasValue)
+                {
+                    count++;
+                }
+
+                return count;
+            }
+        }
+
+        /// <summary>
+        /// Returns a flat list of the global chromatograms.
+        /// This list is always in the following order:
+        /// Total Ion Current
+        /// Base Peak
+        /// all qc traces
+        /// </summary>
+        public IList<ChromKey> ListChromKeys()
+        {
+            var list = new List<ChromKey>();
+            foreach (var possibleGlobalIndex in new[] { TicChromatogramIndex, BpcChromatogramIndex })
+            {
+                if (!possibleGlobalIndex.HasValue)
+                    continue;
+                int globalIndex = possibleGlobalIndex.Value;
+                list.Add(ChromKey.FromId(GetChromatogramId(globalIndex, out _), false));
+            }
+
+            foreach (var qcTrace in QcTraces)
+            {
+                list.Add(ChromKey.FromQcTrace(qcTrace));
+            }
+            Assume.AreEqual(list.Count, ChromatogramCount);
+            return list;
+        }
+
+        /// <summary>
+        /// Gets the chromatogram data for the chromatogram at a particular position in the list
+        /// returned by <see cref="ListChromKeys"/>
+        /// </summary>
+        public bool GetChromatogramAt(int index, out float[] times, out float[] intensities)
+        {
+            if (TicChromatogramIndex.HasValue)
+            {
+                if (index == 0)
+                {
+                    return ReadChromatogramFromDataFile(TicChromatogramIndex.Value, out times, out intensities);
+                }
+
+                index--;
+            }
+
+            if (BpcChromatogramIndex.HasValue)
+            {
+                if (index == 0)
+                {
+                    return ReadChromatogramFromDataFile(BpcChromatogramIndex.Value, out times, out intensities);
+                }
+
+                index--;
+            }
+
+            if (index >= 0 && index < QcTraces.Count)
+            {
+                var qcTrace = QcTraces[index];
+                times = MsDataFileImpl.ToFloatArray(qcTrace.Times);
+                intensities = MsDataFileImpl.ToFloatArray(qcTrace.Intensities);
+                return true;
+            }
+
+            times = intensities = null;
+            return false;
+        }
     }
 
     internal sealed class ChromatogramDataProvider : ChromDataProvider
@@ -299,10 +372,10 @@ public ChromatogramDataProvider(MsDataFileImpl dataFile,
 //                _chromIds.Add(new ChromKeyProviderIdPair(ChromKey.FromId(_globalChromatogramExtractor.GetChromatogramId(globalIndex, out int indexId), false), globalIndex));
 //            }
 
-            foreach (var qcTracePair in _globalChromatogramExtractor.QcTraceByIndex)
+            foreach (var qcTrace in _globalChromatogramExtractor.QcTraces)
             {
-                _chromIndices[qcTracePair.Key] = qcTracePair.Key;
-                _chromIds.Add(new ChromKeyProviderIdPair(ChromKey.FromQcTrace(qcTracePair.Value), qcTracePair.Key));
+                _chromIndices[qcTrace.Index] = qcTrace.Index;
+                _chromIds.Add(new ChromKeyProviderIdPair(ChromKey.FromQcTrace(qcTrace), qcTrace.Index));
             }
 
             // CONSIDER(kaipot): Some way to support mzML files converted from MIDAS wiff files

pwiz_tools/Skyline/Model/Results/SpectraChromDataProvider.cs

@@ -556,34 +556,26 @@ public override IEnumerable<ChromKeyProviderIdPair> ChromIds
                 var chromIds = new List<ChromKeyProviderIdPair>(_collectors.ChromKeys.Count);
                 for (int i = 0; i < _collectors.ChromKeys.Count; i++)
                     chromIds.Add(new ChromKeyProviderIdPair(_collectors.ChromKeys[i], i));
+                VerifyGlobalChromatograms(chromIds);
+                return chromIds;
+            }
+        }
 
-                // The global chromatograms (TIC, Base Peak)  and QC traces are always at the end of the list
-                // of ChromIds.
-                _globalChromatogramExtractor.IndexOffset =
-                    chromIds.Count - _globalChromatogramExtractor.GlobalChromatogramIndexes.Count -
-                    _globalChromatogramExtractor.QcTraceByIndex.Count;
-
-                // Verify that the TIC and QC chromatograms are at the indexes where they are expected
-                for (int chromIndex = _globalChromatogramExtractor.IndexOffset; chromIndex < chromIds.Count; chromIndex++)
+        private void VerifyGlobalChromatograms(IList<ChromKeyProviderIdPair> chromIds)
+        {
+            var globalChromKeys = _globalChromatogramExtractor.ListChromKeys();
+            int indexFirstGlobalChromatogram = chromIds.Count - globalChromKeys.Count;
+            for (int relativeIndex = 0; relativeIndex < globalChromKeys.Count; relativeIndex++)
+            {
+                int absoluteIndex = indexFirstGlobalChromatogram + relativeIndex;
+                var expectedChromKey = globalChromKeys[relativeIndex];
+                var actualChromKey = chromIds[absoluteIndex].Key;
+                if (!Equals(expectedChromKey, actualChromKey))
                 {
-                    var chromKey = chromIds[chromIndex].Key;
-                    Assume.AreEqual(SignedMz.ZERO, chromKey.Precursor);
-                    var globalChromIndex = chromIndex - _globalChromatogramExtractor.IndexOffset;
-                    if (_globalChromatogramExtractor.QcTraceByIndex.TryGetValue(globalChromIndex, out var qcTrace))
-                    {
-                        Assume.AreEqual(ChromExtractor.qc, chromKey.Extractor);
-                        Assume.AreEqual(qcTrace.Name, chromKey.ChromatogramGroupId?.QcTraceName);
-                    }
-                    else if (globalChromIndex == _globalChromatogramExtractor.TicChromatogramIndex)
-                    {
-                        Assume.AreEqual(ChromExtractor.summed, chromKey.Extractor);
-                    }
-                    else if (globalChromIndex == _globalChromatogramExtractor.BpcChromatogramIndex)
-                    {
-                        Assume.AreEqual(ChromExtractor.base_peak, chromKey.Extractor);
-                    }
+                    var message = string.Format(@"ChromKey mismatch at position {0}. Expected: {1} Actual: {2}",
+                        absoluteIndex, expectedChromKey, actualChromKey);
+                    Assume.Fail(message);
                 }
-                return chromIds;
             }
         }
 
@@ -631,7 +623,10 @@ public override bool GetChromatogram(int id, ChromatogramGroupId chromatogramGro
             extra = null;
             if (SignedMz.ZERO.Equals(chromKey?.Precursor ?? SignedMz.ZERO))
             {
-                if (_globalChromatogramExtractor.GetChromatogram(id, out float[] times, out float[] intensities))
+                int indexFirstGlobalChromatogram =
+                    _collectors.ChromKeys.Count - _globalChromatogramExtractor.ChromatogramCount;
+                int indexInGlobalChromatogramExtractor = id - indexFirstGlobalChromatogram;
+                if (_globalChromatogramExtractor.GetChromatogramAt(indexInGlobalChromatogramExtractor, out float[] times, out float[] intensities))
                 {
                     timeIntensities = new TimeIntensities(times, intensities, null, null);
                     extra = new ChromExtra(0, 0);

pwiz_tools/Skyline/Model/Results/SpectrumFilter.cs

@@ -343,18 +343,7 @@ public SpectrumFilter(SrmDocument document, MsDataFileUri msDataFileUri, IFilter
 
                 if (gce != null)
                 {
-                    foreach (var possibleGlobalIndex in new [] { gce.TicChromatogramIndex, gce.BpcChromatogramIndex })
-                    {
-                        if (!possibleGlobalIndex.HasValue)
-                            continue;
-                        int globalIndex = possibleGlobalIndex.Value;
-                        listChromKeyFilterIds.Add(ChromKey.FromId(gce.GetChromatogramId(globalIndex, out int indexId), false));
-                    }
-
-                    foreach (var qcTracePair in gce.QcTraceByIndex)
-                    {
-                        listChromKeyFilterIds.Add(ChromKey.FromQcTrace(qcTracePair.Value));
-                    }
+                    listChromKeyFilterIds.AddRange(gce.ListChromKeys());
                 }
 
                 _productChromKeys = listChromKeyFilterIds.ToArray();

pwiz_tools/Skyline/TestFunctional/QcTraceChromatogramTest.cs

@@ -0,0 +1,68 @@
+/*
+ * Original author: Nicholas Shulman <nicksh .at. u.washington.edu>,
+ *                  MacCoss Lab, Department of Genome Sciences, UW
+ *
+ * Copyright 2023 University of Washington - Seattle, WA
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+using System.Collections.Generic;
+using System.Linq;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using pwiz.ProteowizardWrapper;
+using pwiz.Skyline.Model.Results;
+using pwiz.SkylineTestUtil;
+
+namespace pwiz.SkylineTestFunctional
+{
+    /// <summary>
+    /// Tests that the QC trace chromatograms are correctly read from the data file.
+    /// </summary>
+    [TestClass]
+    public class QcTraceChromatogramTest : AbstractFunctionalTest
+    {
+        [TestMethod]
+        public void TestQcTraceChromatograms()
+        {
+            TestFilesZip = @"TestFunctional\QcTraceChromatogramTest.zip";
+            RunFunctionalTest();
+        }
+
+        protected override void DoTest()
+        {
+            var expectedQcTraceNames = new List<string> { "Pump Pressure 1", "Pump Pressure 2" };
+            RunUI(()=>SkylineWindow.OpenFile(TestFilesDir.GetTestPath("QcTraceChromatogramTest.sky")));
+            ImportResultsFile(TestFilesDir.GetTestPath("QcTraceChromatogramTest.mzML"));
+            var measuredResults = SkylineWindow.Document.MeasuredResults;
+            Assert.IsNotNull(measuredResults);
+            Assert.AreEqual(1, measuredResults.Chromatograms.Count);
+            Assert.IsTrue(measuredResults.TryLoadAllIonsChromatogram(measuredResults.Chromatograms[0], ChromExtractor.qc, true, out var qcTraceChromatogramGroups));
+            var qcTraceNames = measuredResults.QcTraceNames.ToList();
+            CollectionAssert.AreEquivalent(expectedQcTraceNames, qcTraceNames);
+            using var msDataFile = new MsDataFileImpl(TestFilesDir.GetTestPath("QcTraceChromatogramTest.mzML"));
+            var qcTraces = msDataFile.GetQcTraces().ToList();
+            Assert.AreEqual(expectedQcTraceNames.Count, qcTraces.Count);
+            foreach (var qcTrace in qcTraces)
+            { 
+                CollectionAssert.Contains(expectedQcTraceNames, qcTrace.Name);
+                var chromatogramGroupInfo = qcTraceChromatogramGroups.Single(chromGroupInfo =>
+                    chromGroupInfo.QcTraceName == qcTrace.Name);
+                Assert.IsNotNull(chromatogramGroupInfo);
+                Assert.AreEqual(1, chromatogramGroupInfo.TransitionPointSets.Count());
+                var timeIntensities = chromatogramGroupInfo.GetTransitionInfo(0, TransformChrom.raw).TimeIntensities;
+                CollectionAssert.AreEqual(MsDataFileImpl.ToFloatArray(qcTrace.Times), timeIntensities.Times.ToList());
+                CollectionAssert.AreEqual(MsDataFileImpl.ToFloatArray(qcTrace.Intensities), timeIntensities.Intensities.ToList());
+            }
+        }
+    }
+}

pwiz_tools/Skyline/TestFunctional/TestFunctional.csproj

@@ -201,6 +201,7 @@
     <Compile Include="MultiInjectionReplicatesTest.cs" />
     <Compile Include="OpenDocWithBackgroundProteomeTest.cs" />
     <Compile Include="PeakAreaPeptideGraphTest.cs" />
+    <Compile Include="QcTraceChromatogramTest.cs" />
     <Compile Include="QcTraceNameTest.cs" />
     <Compile Include="RemoveHeavyModificationTest.cs" />
     <Compile Include="RemoveNeutralLossModificationTest.cs" />