From d2e5b98e51c779261f90612d066bc047f7d38a24 Mon Sep 17 00:00:00 2001 From: trishorts Date: Wed, 7 Oct 2026 13:07:33 -0500 Subject: [PATCH 1/2] Quantify a design with a biorep gap, with a warning, instead of refusing it A condition whose biological replicates are not numbered 1..N (a lost sample, or study-wide numbering such as control 1-2 / treated 3-4) was refused as "biorep N is missing!", and the search skipped quantification. mzLib #1426 keeps the SDRF's numbers as written, so --sdrfDesign now writes such designs. GetErrorsInExperimentalDesign now checks fractions and techreps within each biorep present; the new GetWarningsInExperimentalDesign names a condition's missing biorep numbers. The search and glyco tasks, the CLI design check and the GUI design window report it. The --sdrfDesign help text no longer says replicates are renumbered. Needs the mzLib release carrying #1422 (normalization and the Bayesian step walk the bioreps that exist) and #1426; the pin bump follows it. Co-Authored-By: Claude Opus 5.5 --- MetaMorpheus/CMD/CommandLineSettings.cs | 2 +- MetaMorpheus/CMD/Program.cs | 13 +- .../EngineLayer/ExperimentalDesign.cs | 41 +++-- .../Views/ExperimentalDesignWindow.xaml.cs | 7 + .../PostGlycoSearchAnalysisTask.cs | 5 + .../SearchTask/PostSearchAnalysisTask.cs | 5 + MetaMorpheus/Test/QuantificationTest.cs | 144 +++++++++++++++++- MetaMorpheus/Test/TestCmd.cs | 20 ++- 8 files changed, 217 insertions(+), 20 deletions(-) diff --git a/MetaMorpheus/CMD/CommandLineSettings.cs b/MetaMorpheus/CMD/CommandLineSettings.cs index a618bba3da..cb99c56b7c 100644 --- a/MetaMorpheus/CMD/CommandLineSettings.cs +++ b/MetaMorpheus/CMD/CommandLineSettings.cs @@ -49,7 +49,7 @@ public class CommandLineSettings [Option("auditData", HelpText = "[Optional] Folder of downloaded data files to check the SDRF's comment[data file] entries against, reported as found or missing. Only meaningful with --auditSdrf.")] public string AuditDataDirectory { get; set; } - [Option("sdrfDesign", HelpText = "[Optional] Path to a label-free SDRF file to turn into an ExperimentalDesign.tsv, which is written beside the first spectra file given with -s, where a run looks for it. Every spectra file must be named exactly by one SDRF row; rows for other files are dropped and reported. Biological replicates are renumbered within each condition and the renumbering is printed; fractions are copied as they are. If the SDRF describes a design MetaMorpheus would reject, nothing is written and every reason is printed. An existing design file is never overwritten. Runs on its own: give -s, but no task or database. -o is ignored, with a warning, because a run reads the design only from beside its first spectra file. Exits 0 when the design was written, 2 for a settings error, 4 when the SDRF could not be read and 5 when the design was refused or a design file is already there.")] + [Option("sdrfDesign", HelpText = "[Optional] Path to a label-free SDRF file to turn into an ExperimentalDesign.tsv, which is written beside the first spectra file given with -s, where a run looks for it. Every spectra file must be named exactly by one SDRF row; rows for other files are dropped and reported. Biological replicates and fractions are copied as the SDRF numbers them; a replicate number is added only where the SDRF gives none, and every added number is printed, as is any gap in a condition's replicate numbers. If the SDRF describes a design MetaMorpheus would reject, nothing is written and every reason is printed. An existing design file is never overwritten. Runs on its own: give -s, but no task or database. -o is ignored, with a warning, because a run reads the design only from beside its first spectra file. Exits 0 when the design was written, 2 for a settings error, 4 when the SDRF could not be read and 5 when the design was refused or a design file is already there.")] public string SdrfDesign { get; set; } [Option("sdrfCondition", HelpText = "[Optional] The SDRF factor value columns the condition is built from, in order; space-delimited. A bare name such as 'genotype' means 'factor value[genotype]'; the 'factor value[' prefix may be given in any case. Values are joined with '_'. Needed when the SDRF has more than one factor value column. Only meaningful with --sdrfDesign.")] diff --git a/MetaMorpheus/CMD/Program.cs b/MetaMorpheus/CMD/Program.cs index da34cf8de3..7a45469e9a 100644 --- a/MetaMorpheus/CMD/Program.cs +++ b/MetaMorpheus/CMD/Program.cs @@ -422,10 +422,15 @@ public static int ResolveExperimentalDesign( string designPath = hasClassicDesign ? pathToExperDesign : pathToTmtDesign; List errors; + var warnings = new List(); if (hasClassicDesign) { - ExperimentalDesign.ReadExperimentalDesign(designPath, startingRawFilenameList, out errors); + var design = ExperimentalDesign.ReadExperimentalDesign(designPath, startingRawFilenameList, out errors); + if (!errors.Any()) + { + warnings = ExperimentalDesign.GetWarningsInExperimentalDesign(design); + } } else { @@ -437,6 +442,10 @@ public static int ResolveExperimentalDesign( if (reportToConsole) { write("Read " + Path.GetFileName(designPath) + " successfully"); + foreach (string warning in warnings) + { + write(warning); + } } return 0; @@ -546,7 +555,7 @@ private static void MyTaskEngine_finishedWritingFileHandler(object sender, Singl /// Refuses rather than writes anything MetaMorpheus would reject, because an invalid design is /// worse than none: a run finding one skips quantification with only a warning. The report is /// printed whatever the verbosity, as for --auditSdrf: it is what was asked for, and it is the - /// only record of a biological-replicate renumbering. + /// only record of a biological replicate number added where the SDRF gives none. /// /// 0 when the design was written; 4 when the SDRF could not be read; 5 when the design /// was refused or a design file is already there. diff --git a/MetaMorpheus/EngineLayer/ExperimentalDesign.cs b/MetaMorpheus/EngineLayer/ExperimentalDesign.cs index a627e892d4..7718a0d6a7 100644 --- a/MetaMorpheus/EngineLayer/ExperimentalDesign.cs +++ b/MetaMorpheus/EngineLayer/ExperimentalDesign.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.IO; using System.Linq; using MassSpectrometry; @@ -112,6 +113,9 @@ public static string WriteExperimentalDesignToFile(List spectra /// /// Checks for errors in the experimental design. Will return null if there are no errors. + /// A gap in a condition's biological replicate numbers is not an error: a replicate number can + /// name a subject across conditions, or a sample that was lost, so it is kept as given and + /// reported by GetWarningsInExperimentalDesign. Fractions and techreps must still run 1..N. /// public static string GetErrorsInExperimentalDesign(List spectraFileInfos) { @@ -121,18 +125,12 @@ public static string GetErrorsInExperimentalDesign(List spectra foreach (var condition in conditions) { var temp = condition.OrderBy(p => p.BiologicalReplicate).ThenBy(p => p.Fraction).ThenBy(p => p.TechnicalReplicate); - int numB = temp.Max(p => p.BiologicalReplicate + 1); - // check bioreps are in order - for (int b = 0; b < numB; b++) + // check each biorep that is present; a missing biorep is a warning, not an error + foreach (int b in temp.Select(p => p.BiologicalReplicate).Distinct()) { var biorepFiles = temp.Where(p => p.BiologicalReplicate == b); - if (!biorepFiles.Any()) - { - return "Condition \"" + condition.Key + "\" biorep " + (b + 1) + " is missing!"; - } - // check fractions are in order int numF = biorepFiles.Max(p => p.Fraction + 1); @@ -169,5 +167,30 @@ public static string GetErrorsInExperimentalDesign(List spectra return null; } + + /// + /// One warning per condition whose biological replicates are not numbered 1..N, naming the + /// missing numbers. Such a design is quantified as numbered; the warning is there because a + /// missing number may be a sample that was lost or not searched. Empty when there is nothing to say. + /// + public static List GetWarningsInExperimentalDesign(List spectraFileInfos) + { + var warnings = new List(); + + foreach (var condition in spectraFileInfos.GroupBy(p => p.Condition)) + { + var present = condition.Select(p => p.BiologicalReplicate + 1).Distinct().OrderBy(b => b).ToList(); + var missing = Enumerable.Range(1, Math.Max(0, present.Max())).Except(present).ToList(); + + if (missing.Any()) + { + warnings.Add("Condition \"" + condition.Key + "\" has biorep " + string.Join(", ", present) + + " but not biorep " + string.Join(", ", missing) + ". The bioreps are quantified as numbered; " + + "a missing number may be a sample that was lost or not searched."); + } + } + + return warnings; + } } } diff --git a/MetaMorpheus/GUI/Views/ExperimentalDesignWindow.xaml.cs b/MetaMorpheus/GUI/Views/ExperimentalDesignWindow.xaml.cs index d434743dd2..ebdc663d8e 100644 --- a/MetaMorpheus/GUI/Views/ExperimentalDesignWindow.xaml.cs +++ b/MetaMorpheus/GUI/Views/ExperimentalDesignWindow.xaml.cs @@ -81,6 +81,13 @@ private void BtnSaveQuant_Click(object sender, RoutedEventArgs e) .Select(p => new SpectraFileInfo(p.FullFilePathWithExtension, p.Condition, int.Parse(p.Biorep) - 1, int.Parse(p.Techrep) - 1, int.Parse(p.Fraction) - 1)) .ToList(); ExperimentalDesign.WriteExperimentalDesignToFile(fileInfos); + + // saved as numbered; said, because a skipped biorep number is often a typo + var warnings = ExperimentalDesign.GetWarningsInExperimentalDesign(fileInfos); + if (warnings.Any()) + { + MessageBox.Show("Experimental design saved.\n\n" + string.Join("\n\n", warnings)); + } } catch (Exception ex) { diff --git a/MetaMorpheus/TaskLayer/GlycoSearchTask/PostGlycoSearchAnalysisTask.cs b/MetaMorpheus/TaskLayer/GlycoSearchTask/PostGlycoSearchAnalysisTask.cs index 16b356b060..8095fb9d4d 100644 --- a/MetaMorpheus/TaskLayer/GlycoSearchTask/PostGlycoSearchAnalysisTask.cs +++ b/MetaMorpheus/TaskLayer/GlycoSearchTask/PostGlycoSearchAnalysisTask.cs @@ -477,6 +477,11 @@ private void QuantificationAnalysis() Warn("Error reading experimental design file: " + errors.First() + ". Skipping quantification"); return; } + + foreach (string warning in ExperimentalDesign.GetWarningsInExperimentalDesign(spectraFileInfo)) + { + Warn(warning); + } } else { diff --git a/MetaMorpheus/TaskLayer/SearchTask/PostSearchAnalysisTask.cs b/MetaMorpheus/TaskLayer/SearchTask/PostSearchAnalysisTask.cs index defa1a8daf..6720409642 100644 --- a/MetaMorpheus/TaskLayer/SearchTask/PostSearchAnalysisTask.cs +++ b/MetaMorpheus/TaskLayer/SearchTask/PostSearchAnalysisTask.cs @@ -944,6 +944,11 @@ private void QuantificationAnalysis() Warn("Error reading experimental design file: " + errors.First() + ". Skipping quantification"); return; } + + foreach (string warning in ExperimentalDesign.GetWarningsInExperimentalDesign(spectraFileInfo)) + { + Warn(warning); + } } else if (Parameters.SearchParameters.Normalize) { diff --git a/MetaMorpheus/Test/QuantificationTest.cs b/MetaMorpheus/Test/QuantificationTest.cs index 3e3b688744..eb5ec9c368 100644 --- a/MetaMorpheus/Test/QuantificationTest.cs +++ b/MetaMorpheus/Test/QuantificationTest.cs @@ -51,7 +51,7 @@ public static void TestExperimentalDesignErrors() string outputFolder = Path.Combine(TestContext.CurrentContext.TestDirectory, @"ExperimentalDesignTest"); Directory.CreateDirectory(outputFolder); - // test non-consecutive bioreps (should produce an error) + // test non-consecutive bioreps (a warning, not an error: the numbers are kept as given) List spectraFiles = new List(); spectraFiles.Add(new SpectraFileInfo(Path.Combine(outputFolder, @"myFile.1.raw"), "condition1", 0, 0, 0)); @@ -65,7 +65,9 @@ public static void TestExperimentalDesignErrors() spectraFiles.Select(p => p.FullFilePathWithExtension).ToList(), out var errors); - Assert.That(errors.Any()); + Assert.That(errors, Is.Empty); + Assert.That(readIn.Select(p => p.BiologicalReplicate), Is.EquivalentTo(new[] { 0, 1, 3 })); + Assert.That(ExperimentalDesign.GetWarningsInExperimentalDesign(readIn), Has.Count.EqualTo(1)); // test non-consecutive fractions (should produce an error) spectraFiles.Clear(); @@ -150,6 +152,144 @@ public static void TestExperimentalDesignErrors() Directory.Delete(outputFolder, true); } + /// + /// A biorep number can name a subject across conditions (study-wide numbering, as an SDRF often + /// gives it) or mark a lost sample, so a gap is kept and warned about rather than refused. + /// Fractions and techreps of the bioreps that are present are still checked. + /// + [Test] + public static void TestBiorepGapIsAWarningNotAnError() + { + static SpectraFileInfo Info(string name, string condition, int oneBasedBiorep, int oneBasedFraction = 1) => + new(name + ".raw", condition, oneBasedBiorep - 1, 0, oneBasedFraction - 1); + + // 1, 2, 4: biorep 3 is named as missing + var gap = new List { Info("a1", "A", 1), Info("a2", "A", 2), Info("a4", "A", 4) }; + Assert.That(ExperimentalDesign.GetErrorsInExperimentalDesign(gap), Is.Null); + Assert.That(ExperimentalDesign.GetWarningsInExperimentalDesign(gap), Is.EqualTo(new[] + { + "Condition \"A\" has biorep 1, 2, 4 but not biorep 3. The bioreps are quantified as numbered; " + + "a missing number may be a sample that was lost or not searched." + })); + + // study-wide numbering: each condition is judged on its own numbers + var studyWide = new List { Info("c1", "control", 1), Info("c2", "control", 2), Info("t3", "treated", 3), Info("t4", "treated", 4) }; + Assert.That(ExperimentalDesign.GetErrorsInExperimentalDesign(studyWide), Is.Null); + var warnings = ExperimentalDesign.GetWarningsInExperimentalDesign(studyWide); + Assert.That(warnings, Has.Count.EqualTo(1)); + Assert.That(warnings[0], Does.StartWith("Condition \"treated\" has biorep 3, 4 but not biorep 1, 2.")); + + // 1..N says nothing + var complete = new List { Info("a1", "A", 1), Info("a2", "A", 2) }; + Assert.That(ExperimentalDesign.GetWarningsInExperimentalDesign(complete), Is.Empty); + + // a biorep after a gap still has its fractions checked + var fractionGap = new List { Info("a1", "A", 1), Info("a4f2", "A", 4, oneBasedFraction: 2) }; + Assert.That(ExperimentalDesign.GetErrorsInExperimentalDesign(fractionGap), Is.EqualTo("Condition \"A\" biorep 4 fraction 1 is missing!")); + } + + /// + /// End to end, with normalization on: a design numbered study-wide (A: 2, 3; B: 4, 5) used to make + /// the search skip quantification with one warning. It is now quantified under the numbers given, + /// and the gaps are warned about. The first condition has no biorep 1, which is the normalization + /// reference before mzLib #1422; without #1422 biorep normalization silently does nothing here. + /// + [Test] + public static void TestSearchQuantifiesADesignWithABiorepGap() + { + string unitTestFolder = Path.Combine(TestContext.CurrentContext.TestDirectory, @"TestSearchQuantifiesADesignWithABiorepGap"); + if (Directory.Exists(unitTestFolder)) + { + Directory.Delete(unitTestFolder, true); + } + _ = Directory.CreateDirectory(unitTestFolder); + + string peptide = "PEPTIDE"; + Protein prot = new(peptide, @"test"); + string dbName = Path.Combine(unitTestFolder, "testDB.fasta"); + UsefulProteomicsDatabases.ProteinDbWriter.WriteFastaDatabase(new List { prot }, dbName, ">"); + + var fileInfos = new List(); + foreach (var (condition, oneBasedBiorep) in new[] { ("A", 2), ("A", 3), ("B", 4), ("B", 5) }) + { + string fullPath = Path.Combine(unitTestFolder, condition + oneBasedBiorep + ".mzML"); + WriteOnePeptideMzml(peptide, 1e6 * oneBasedBiorep, fullPath); + fileInfos.Add(new SpectraFileInfo(fullPath, condition, oneBasedBiorep - 1, 0, 0)); + } + _ = ExperimentalDesign.WriteExperimentalDesignToFile(fileInfos); + + var warnings = new List(); + EventHandler handler = (o, e) => warnings.Add(e.S); + MetaMorpheusTask.WarnHandler += handler; + try + { + SearchTask task = new SearchTask(); + task.SearchParameters.Normalize = true; + task.RunTask(unitTestFolder, new List { new DbForTask(dbName, false) }, fileInfos.Select(p => p.FullFilePathWithExtension).ToList(), ""); + } + finally + { + MetaMorpheusTask.WarnHandler -= handler; + } + + Assert.That(warnings, Has.None.Contain("Skipping quantification")); + Assert.That(warnings, Has.Some.StartWith("Condition \"A\" has biorep 2, 3 but not biorep 1.")); + Assert.That(warnings, Has.Some.StartWith("Condition \"B\" has biorep 4, 5 but not biorep 1, 2, 3.")); + + // the protein table's sample columns carry the numbers the design gives + string proteinTable = Path.Combine(unitTestFolder, "AllQuantifiedProteinGroups.tsv"); + Assert.That(File.Exists(proteinTable)); + var proteinHeader = File.ReadAllLines(proteinTable)[0].Split('\t'); + Assert.That(proteinHeader.Where(h => h.StartsWith("Intensity_")), + Is.EquivalentTo(new[] { "Intensity_A_2", "Intensity_A_3", "Intensity_B_4", "Intensity_B_5" })); + + // and every file's peptide was quantified and normalized: the files were written at 2, 3, 4 + // and 5 times one intensity, and normalization brings this one peptide back to one value + string[] lines = File.ReadAllLines(Path.Combine(unitTestFolder, "AllQuantifiedPeptides.tsv")); + var header = lines[0].Split('\t').ToList(); + var intensities = new[] { "Intensity_A2", "Intensity_A3", "Intensity_B4", "Intensity_B5" } + .Select(column => double.Parse(lines[1].Split('\t')[header.IndexOf(column)])) + .ToList(); + Assert.That(intensities, Has.All.GreaterThan(0)); + Assert.That(intensities.Max() / intensities.Min(), Is.LessThan(1.01)); + + Directory.Delete(unitTestFolder, true); + } + + /// + /// An mzML holding one MS1 scan of the peptide's isotopic envelope at the given intensity, and one + /// MS2 scan of its b and y ions. The same scans TestProteinQuantFileHeaders writes. + /// + private static void WriteOnePeptideMzml(string peptide, double ionIntensity, string fullPath) + { + MsDataScan[] scans = new MsDataScan[2]; + + ChemicalFormula cf = new Proteomics.AminoAcidPolymer.Peptide(peptide).GetChemicalFormula(); + IsotopicDistribution dist = IsotopicDistribution.GetDistribution(cf, 0.125, 1e-8); + double[] mz = dist.Masses.Select(v => v.ToMz(1)).ToArray(); + double[] intensities = dist.Intensities.Select(v => v * ionIntensity).ToArray(); + + scans[0] = new MsDataScan(massSpectrum: new MzSpectrum(mz, intensities, false), oneBasedScanNumber: 1, msnOrder: 1, isCentroid: true, + polarity: Polarity.Positive, retentionTime: 1.0, scanWindowRange: new MzRange(400, 1600), scanFilter: "f", + mzAnalyzer: MZAnalyzerType.Orbitrap, totalIonCurrent: intensities.Sum(), injectionTime: 1.0, noiseData: null, nativeId: "scan=1"); + + var pep = new PeptideWithSetModifications(peptide, new Dictionary()); + List frags = new List(); + pep.Fragment(DissociationType.HCD, FragmentationTerminus.Both, frags); + double[] mz2 = frags.Select(v => v.NeutralMass.ToMz(1)).ToArray(); + double[] intensities2 = frags.Select(v => 1e6).ToArray(); + + scans[1] = new MsDataScan(massSpectrum: new MzSpectrum(mz2, intensities2, false), oneBasedScanNumber: 2, msnOrder: 2, isCentroid: true, + polarity: Polarity.Positive, retentionTime: 1.01, scanWindowRange: new MzRange(100, 1600), scanFilter: "f", + mzAnalyzer: MZAnalyzerType.Orbitrap, totalIonCurrent: intensities.Sum(), injectionTime: 1.0, noiseData: null, nativeId: "scan=2", selectedIonMz: pep.MonoisotopicMass.ToMz(1), + selectedIonChargeStateGuess: 1, selectedIonIntensity: 1e6, isolationMZ: pep.MonoisotopicMass.ToMz(1), isolationWidth: 1.5, dissociationType: DissociationType.HCD, + oneBasedPrecursorScanNumber: 1, selectedIonMonoisotopicGuessMz: pep.MonoisotopicMass.ToMz(1), hcdEnergy: "35"); + + Readers.MzmlMethods.CreateAndWriteMyMzmlWithCalibratedSpectra( + new GenericMsDataFile(scans, new SourceFile(@"scan number only nativeID format", "mzML format", null, "SHA-1", @"C:\fake.mzML", null)), + fullPath, false); + } + [Test] [TestCase(false, 2, 1, 1)] [TestCase(true, 2, 3, 1)] diff --git a/MetaMorpheus/Test/TestCmd.cs b/MetaMorpheus/Test/TestCmd.cs index 0b7735b658..22b00977a7 100644 --- a/MetaMorpheus/Test/TestCmd.cs +++ b/MetaMorpheus/Test/TestCmd.cs @@ -1290,11 +1290,12 @@ public static void TestSdrfDesignIsReadBackByMetaMorpheusWithoutError() } /// - /// Study-wide replicate numbering (control 1-2, treated 3-4) is rejected by MetaMorpheus as - /// "biorep 1 is missing". It is ranked within each condition, and the renumbering is printed. + /// Study-wide replicate numbering (control 1-2, treated 3-4) is kept as the SDRF gives it, and + /// the gap is reported. A run reads the design, warns about the gap and quantifies, rather than + /// refusing it as "biorep 1 is missing". /// [Test] - public static void TestSdrfDesignRenumbersStudyWideReplicatesAndSaysSo() + public static void TestSdrfDesignKeepsStudyWideReplicatesAndSaysSo() { var (sdrf, data) = WriteLabelFreeSdrf( ("c1.raw", "WT", "DMSO", 1, 1), ("c2.raw", "WT", "DMSO", 2, 1), @@ -1303,12 +1304,19 @@ public static void TestSdrfDesignRenumbersStudyWideReplicatesAndSaysSo() var written = new StringWriter(); Assert.That(Program.WriteDesignFromSdrf(settings, written), Is.EqualTo(0), written.ToString()); - Assert.That(written.ToString(), Does.Contain("biological replicates renumbered 3 -> 1, 4 -> 2")); + Assert.That(written.ToString(), Does.Contain("Condition 'FA': biological replicates 3, 4, kept as the SDRF numbers them.")); + Assert.That(written.ToString(), Does.Not.Contain("renumbered")); + var searched = settings.Spectra.Select(Path.GetFullPath).ToList(); var readBack = ExperimentalDesign.ReadExperimentalDesign( - Path.Combine(data, GlobalVariables.ExperimentalDesignFileName), settings.Spectra.Select(Path.GetFullPath).ToList(), out var errors); + Path.Combine(data, GlobalVariables.ExperimentalDesignFileName), searched, out var errors); Assert.That(errors, Is.Empty); - Assert.That(readBack.Where(f => f.Condition == "FA").Select(f => f.BiologicalReplicate), Is.EquivalentTo(new[] { 0, 1 })); + Assert.That(readBack.Where(f => f.Condition == "FA").Select(f => f.BiologicalReplicate), Is.EquivalentTo(new[] { 2, 3 })); + + var console = new List(); + Assert.That(Program.ResolveExperimentalDesign(data, searched, normalizationRequested: true, reportToConsole: true, write: console.Add), + Is.EqualTo(0)); + Assert.That(console, Has.Some.StartWith("Condition \"FA\" has biorep 3, 4 but not biorep 1, 2.")); } /// From fd4742c154ac6c057dc39272b10603d82769a30f Mon Sep 17 00:00:00 2001 From: trishorts Date: Wed, 7 Oct 2026 13:46:52 -0500 Subject: [PATCH 2/2] Warn on fraction and techrep gaps too, record every gap warning in results.txt, and give the TMT window's rules to EngineLayer Label-free: GetErrorsInExperimentalDesign now refuses only a duplicate (two files at one condition, biorep, fraction and techrep); a gap in fractions or techreps is a warning, like a gap in bioreps. TMT (#2793): the TMT design window's ValidateDesign rules move into TmtExperimentalDesign.GetErrorsInDesign / GetWarningsInDesign, so a test reaches them; a gap in a plex's fractions or techreps is a warning there and, for the first time, at search time. Every gap warning is written with Warn and into results.txt (AddTaskSummaryText), in the search, glyco and TMT paths, and is worded as mzLib's SdrfLabelFreeDesign notes the same gap (#1426): "Condition "A" biorep 1: fractions 1, 3, quantified as numbered; fraction 2 is not in the design." GlycoTestWithBadExperimentalDesignFile's bad design is now a duplicate, since its techrep gap is valid. Co-Authored-By: Claude Opus 5.5 --- MetaMorpheus/CMD/Program.cs | 6 +- .../EngineLayer/ExperimentalDesign.cs | 104 ++++++------ .../EngineLayer/TmtExperimentalDesign.cs | 82 +++++++++ .../Views/TmtExperimentalDesignWindow.xaml.cs | 70 ++------ .../PostGlycoSearchAnalysisTask.cs | 2 + .../SearchTask/PostSearchAnalysisTask.cs | 9 + .../Test/MultiplexQuantificationTests.cs | 79 +++++++++ MetaMorpheus/Test/QuantificationTest.cs | 157 +++++++++++++----- MetaMorpheus/Test/TestCmd.cs | 39 ++++- MetaMorpheus/Test/TestOGlyco.cs | 9 +- .../Test/TmtExperimentalDesignTests.cs | 51 ++++++ 11 files changed, 448 insertions(+), 160 deletions(-) diff --git a/MetaMorpheus/CMD/Program.cs b/MetaMorpheus/CMD/Program.cs index 7a45469e9a..45b6195f3d 100644 --- a/MetaMorpheus/CMD/Program.cs +++ b/MetaMorpheus/CMD/Program.cs @@ -434,7 +434,11 @@ public static int ResolveExperimentalDesign( } else { - TmtExperimentalDesign.Read(designPath, startingRawFilenameList, out errors); + var tmtFiles = TmtExperimentalDesign.Read(designPath, startingRawFilenameList, out errors); + if (!errors.Any()) + { + warnings = TmtExperimentalDesign.GetWarningsInDesign(tmtFiles); + } } if (!errors.Any()) diff --git a/MetaMorpheus/EngineLayer/ExperimentalDesign.cs b/MetaMorpheus/EngineLayer/ExperimentalDesign.cs index 7718a0d6a7..288137bd28 100644 --- a/MetaMorpheus/EngineLayer/ExperimentalDesign.cs +++ b/MetaMorpheus/EngineLayer/ExperimentalDesign.cs @@ -113,84 +113,76 @@ public static string WriteExperimentalDesignToFile(List spectra /// /// Checks for errors in the experimental design. Will return null if there are no errors. - /// A gap in a condition's biological replicate numbers is not an error: a replicate number can - /// name a subject across conditions, or a sample that was lost, so it is kept as given and - /// reported by GetWarningsInExperimentalDesign. Fractions and techreps must still run 1..N. + /// The one error is a duplicate: two files at the same condition, biorep, fraction and techrep. + /// A gap in biorep, fraction or techrep numbers is not an error. The numbers are kept as given + /// (a biorep number can name a subject across conditions, and a gap can be a lost sample) and + /// reported by GetWarningsInExperimentalDesign. /// public static string GetErrorsInExperimentalDesign(List spectraFileInfos) { - // check for correct iteration of integer values and duplicates - var conditions = spectraFileInfos.GroupBy(p => p.Condition); - - foreach (var condition in conditions) + var duplicate = spectraFileInfos + .GroupBy(p => (p.Condition, p.BiologicalReplicate, p.Fraction, p.TechnicalReplicate)) + .Where(g => g.Count() > 1) + .Select(g => g.Key) + .OrderBy(k => k.Condition, StringComparer.Ordinal).ThenBy(k => k.BiologicalReplicate).ThenBy(k => k.Fraction).ThenBy(k => k.TechnicalReplicate) + .FirstOrDefault(); + + if (duplicate != default) { - var temp = condition.OrderBy(p => p.BiologicalReplicate).ThenBy(p => p.Fraction).ThenBy(p => p.TechnicalReplicate); - - // check each biorep that is present; a missing biorep is a warning, not an error - foreach (int b in temp.Select(p => p.BiologicalReplicate).Distinct()) - { - var biorepFiles = temp.Where(p => p.BiologicalReplicate == b); - - // check fractions are in order - int numF = biorepFiles.Max(p => p.Fraction + 1); - - for (int f = 0; f < numF; f++) - { - var fractionFiles = biorepFiles.Where(p => p.Fraction == f); - - if (!fractionFiles.Any()) - { - return "Condition \"" + condition.Key + "\" biorep " + (b + 1) + " fraction " + (f + 1) + " is missing!"; - } - - // check techreps are in order - int numT = fractionFiles.Max(p => p.TechnicalReplicate + 1); - - for (int t = 0; t < numT; t++) - { - var techrepFiles = fractionFiles.Where(p => p.TechnicalReplicate == t); - - if (!techrepFiles.Any()) - { - return "Condition \"" + condition.Key + "\" biorep " + (b + 1) + " fraction " + (f + 1) + " techrep " + (t + 1) + " is missing!"; - } - - if (techrepFiles.Count() > 1) - { - return "Duplicates are not allowed:\n" + - "Condition \"" + condition.Key + "\" biorep " + (b + 1) + " fraction " + (f + 1) + " techrep " + (t + 1); - } - } - } - } + return "Duplicates are not allowed:\n" + + "Condition \"" + duplicate.Condition + "\" biorep " + (duplicate.BiologicalReplicate + 1) + + " fraction " + (duplicate.Fraction + 1) + " techrep " + (duplicate.TechnicalReplicate + 1); } return null; } /// - /// One warning per condition whose biological replicates are not numbered 1..N, naming the - /// missing numbers. Such a design is quantified as numbered; the warning is there because a - /// missing number may be a sample that was lost or not searched. Empty when there is nothing to say. + /// One warning for each place the numbering has a gap, naming the missing numbers: a condition's + /// bioreps, a biorep's fractions, and a fraction's techreps, each expected to run 1..N. Such a design + /// is quantified as numbered; the warning is there because a missing number may be a sample or + /// file that was lost or not searched. Empty when there is nothing to say. /// public static List GetWarningsInExperimentalDesign(List spectraFileInfos) { var warnings = new List(); - foreach (var condition in spectraFileInfos.GroupBy(p => p.Condition)) + foreach (var condition in spectraFileInfos.GroupBy(p => p.Condition).OrderBy(c => c.Key, StringComparer.Ordinal)) { - var present = condition.Select(p => p.BiologicalReplicate + 1).Distinct().OrderBy(b => b).ToList(); - var missing = Enumerable.Range(1, Math.Max(0, present.Max())).Except(present).ToList(); + string conditionLabel = "Condition \"" + condition.Key + "\""; + AddGapWarning(warnings, conditionLabel, "biological replicate", condition.Select(p => p.BiologicalReplicate)); - if (missing.Any()) + foreach (var biorep in condition.GroupBy(p => p.BiologicalReplicate).OrderBy(b => b.Key)) { - warnings.Add("Condition \"" + condition.Key + "\" has biorep " + string.Join(", ", present) + - " but not biorep " + string.Join(", ", missing) + ". The bioreps are quantified as numbered; " + - "a missing number may be a sample that was lost or not searched."); + string biorepLabel = conditionLabel + " biorep " + (biorep.Key + 1); + AddGapWarning(warnings, biorepLabel, "fraction", biorep.Select(p => p.Fraction)); + + foreach (var fraction in biorep.GroupBy(p => p.Fraction).OrderBy(f => f.Key)) + { + AddGapWarning(warnings, biorepLabel + " fraction " + (fraction.Key + 1), "technical replicate", fraction.Select(p => p.TechnicalReplicate)); + } } } return warnings; } + + /// + /// Adds a warning when the given zero-based numbers, read one-based, are not 1..N. Worded as mzLib's + /// SdrfLabelFreeDesign notes the same gap ("Condition 'A' biorep 1: fractions 1, 3, kept as the SDRF + /// numbers them."), so the SDRF report and the run say one thing. + /// + private static void AddGapWarning(List warnings, string owner, string level, IEnumerable zeroBasedNumbers) + { + var present = zeroBasedNumbers.Select(n => n + 1).Distinct().OrderBy(n => n).ToList(); + var missing = Enumerable.Range(1, Math.Max(0, present.Max())).Except(present).ToList(); + + if (missing.Any()) + { + warnings.Add(owner + ": " + level + "s " + string.Join(", ", present) + ", quantified as numbered; " + + level + (missing.Count == 1 ? " " : "s ") + string.Join(", ", missing) + (missing.Count == 1 ? " is" : " are") + " not in the design. " + + "A missing number may be a sample or file that was lost or not searched."); + } + } } } diff --git a/MetaMorpheus/EngineLayer/TmtExperimentalDesign.cs b/MetaMorpheus/EngineLayer/TmtExperimentalDesign.cs index b946792481..a7e275fff5 100644 --- a/MetaMorpheus/EngineLayer/TmtExperimentalDesign.cs +++ b/MetaMorpheus/EngineLayer/TmtExperimentalDesign.cs @@ -60,6 +60,88 @@ private static string ValidateUniqueSampleBioFracTech(IEnumerable<(string Plex, return string.Join(Environment.NewLine, msgs); } + /// + /// The per-file rules of a TMT design, scoped to a plex because a plex is its own labelling + /// experiment: every file has a plex, fractions and technical replicates are >= 1, and no plex + /// holds two files at one (fraction, technical replicate). Null when there is no error. A gap in + /// fraction or technical replicate numbers is not an error; see . + /// Lifted from the TMT design window (#2793), so the window and a test check the same rules. + /// + public static string GetErrorsInDesign(IEnumerable files) + { + var fileList = files.ToList(); + + // A file with no plex passes every per-plex check, and then does not appear in the written + // design at all, because the design is built by grouping on plex. + var unplexed = fileList.Where(f => string.IsNullOrWhiteSpace(f.Plex)).ToList(); + if (unplexed.Any()) + { + return "Every file must be assigned a Plex. Missing for: " + + string.Join(", ", unplexed.Select(f => Path.GetFileName(f.FullFilePathWithExtension))); + } + + if (fileList.Any(f => f.TechnicalReplicate < 1)) + return "Technical Replicate values must be >= 1."; + + if (fileList.Any(f => f.Fraction < 1)) + return "Fraction values must be >= 1."; + + // Scoped to one plex: two files in different plexes both at (Fraction 1, Technical Replicate 1) + // is the correct design for a two-plex run with one fraction and one technical replicate. + foreach (var plexGroup in fileList.GroupBy(f => f.Plex.Trim(), StringComparer.OrdinalIgnoreCase)) + { + var duplicatePair = plexGroup.GroupBy(f => (f.Fraction, f.TechnicalReplicate)) + .FirstOrDefault(g => g.Count() > 1); + if (duplicatePair != null) + return $"Plex {plexGroup.Key}: duplicate Fraction/Technical Replicate combination: Fraction {duplicatePair.Key.Fraction}, Technical Replicate {duplicatePair.Key.TechnicalReplicate}."; + } + + return null; + } + + /// + /// One warning for each place a plex's numbering has a gap, naming the missing numbers: its + /// fractions, and each fraction's technical replicates, each expected to run 1..N. Such a design + /// is quantified as numbered; the warning is there because a missing number may be a file that + /// was lost or not searched. Empty when there is nothing to say. + /// + public static List GetWarningsInDesign(IEnumerable files) + { + var warnings = new List(); + + foreach (var plexGroup in files.Where(f => !string.IsNullOrWhiteSpace(f.Plex)) + .GroupBy(f => f.Plex.Trim(), StringComparer.OrdinalIgnoreCase) + .OrderBy(g => g.Key, StringComparer.OrdinalIgnoreCase)) + { + string plexLabel = $"Plex {plexGroup.Key}"; + AddGapWarning(warnings, plexLabel, "fraction", plexGroup.Select(f => f.Fraction)); + + foreach (var fraction in plexGroup.GroupBy(f => f.Fraction).OrderBy(g => g.Key)) + { + AddGapWarning(warnings, $"{plexLabel} fraction {fraction.Key}", "technical replicate", + fraction.Select(f => f.TechnicalReplicate)); + } + } + + return warnings; + } + + /// + /// Adds a warning when the given one-based numbers are not 1..N. + /// + private static void AddGapWarning(List warnings, string owner, string level, IEnumerable oneBasedNumbers) + { + var present = oneBasedNumbers.Distinct().OrderBy(n => n).ToList(); + var missing = Enumerable.Range(1, Math.Max(0, present.Max())).Except(present).ToList(); + + if (missing.Any()) + { + warnings.Add($"{owner}: {level}s {string.Join(", ", present)}, quantified as numbered; " + + $"{level}{(missing.Count == 1 ? "" : "s")} {string.Join(", ", missing)} {(missing.Count == 1 ? "is" : "are")} not in the design. " + + "A missing number may be a file that was lost or not searched."); + } + } + // RETURN: files where each file carries its plex annotations /// /// The files this run actually searched, used to ignore design rows naming anything else. diff --git a/MetaMorpheus/GUI/Views/TmtExperimentalDesignWindow.xaml.cs b/MetaMorpheus/GUI/Views/TmtExperimentalDesignWindow.xaml.cs index c4b627c7e9..afed92530e 100644 --- a/MetaMorpheus/GUI/Views/TmtExperimentalDesignWindow.xaml.cs +++ b/MetaMorpheus/GUI/Views/TmtExperimentalDesignWindow.xaml.cs @@ -83,62 +83,15 @@ private string ValidateDesign() CommitPendingEdits(); - // A file with no plex passes every check below -- they are all scoped to a plex -- and - // then does not appear in the written design at all, because the design is built by - // grouping on plex. Silently dropping a file the user can see in the grid is worse than - // refusing to save. - var unplexed = _rows.Where(r => string.IsNullOrWhiteSpace(r.Plex)).ToList(); - if (unplexed.Any()) - { - return "Every file must be assigned a Plex. Missing for: " - + string.Join(", ", unplexed.Select(r => Path.GetFileName(r.FilePath))); - } - - if (_rows.Any(r => r.TechnicalReplicate < 1)) - return "Technical Replicate values must be >= 1."; - - if (_rows.Any(r => r.Fraction < 1)) - return "Fraction values must be >= 1."; - - // Every check below is scoped to one plex, because _rows spans plexes and a plex is its - // own labelling experiment. Unscoped, two files in different plexes both sitting at - // (Fraction 1, Technical Replicate 1) -- which is the correct design for a two-plex run - // with one fraction and one technical replicate, and the value the TmtDesignRow - // constructor assigns -- read as a duplicate and Save was refused. It also broke the - // round trip: SeedFromDesignFiles stores fraction, replicate and plex per file, so a - // two-plex design the parser accepts loaded into this grid and then could not be saved - // back out. TmtExperimentalDesign keys its own per-file state on plex the same way. - foreach (var plexGroup in _rows.GroupBy(r => r.Plex.Trim(), StringComparer.OrdinalIgnoreCase)) - { - string plexLabel = $"Plex {plexGroup.Key}"; - - var distinctFractions = plexGroup.Select(r => r.Fraction).Distinct().OrderBy(i => i).ToList(); - if (distinctFractions.First() != 1) - return $"{plexLabel}: fractions must start at 1."; - int maxFraction = distinctFractions.Last(); - for (int i = 1; i <= maxFraction; i++) - if (!distinctFractions.Contains(i)) - return $"{plexLabel}: missing fraction number {i} in distinct set."; - - foreach (var grp in plexGroup.GroupBy(r => r.Fraction)) - { - var techs = grp.Select(r => r.TechnicalReplicate).Distinct().OrderBy(t => t).ToList(); - if (techs.First() != 1) - return $"{plexLabel}, fraction {grp.Key}: technical replicates must start at 1."; - int maxTech = techs.Last(); - for (int t = 1; t <= maxTech; t++) - if (!techs.Contains(t)) - return $"{plexLabel}, fraction {grp.Key}: missing technical replicate {t}."; - } + // The rules live in EngineLayer (#2793), where a test can reach them. + return TmtExperimentalDesign.GetErrorsInDesign(RowsAsFiles()); + } - var duplicatePair = plexGroup.GroupBy(r => (r.Fraction, r.TechnicalReplicate)) - .FirstOrDefault(g => g.Count() > 1); - if (duplicatePair != null) - return $"{plexLabel}: duplicate Fraction/Technical Replicate combination: Fraction {duplicatePair.Key.Fraction}, Technical Replicate {duplicatePair.Key.TechnicalReplicate}."; - } + /// A gap in fraction or technical replicate numbers, said but not refused. + private List DesignWarnings() => TmtExperimentalDesign.GetWarningsInDesign(RowsAsFiles()); - return null; - } + private List RowsAsFiles() => + _rows.Select(r => new TmtFileInfo(r.FilePath, r.Plex, r.Fraction, r.TechnicalReplicate, null)).ToList(); #endregion #region Editing @@ -222,7 +175,10 @@ private void ValidateButton_Click(object sender, RoutedEventArgs e) { CommitPendingEdits(); var err = ValidateDesign(); - MessageBox.Show(err ?? "Validation passed.", + var warnings = err == null ? DesignWarnings() : new List(); + MessageBox.Show(err ?? (warnings.Any() + ? "Validation passed, with warnings:" + Environment.NewLine + Environment.NewLine + string.Join(Environment.NewLine + Environment.NewLine, warnings) + : "Validation passed."), err == null ? "OK" : "Validation Error"); } @@ -288,9 +244,11 @@ private void SaveButton_Click(object sender, RoutedEventArgs e) else { int plexCount = design.Select(f => f.Plex).Distinct(StringComparer.OrdinalIgnoreCase).Count(); + var warnings = DesignWarnings(); MessageBox.Show( $"Saved TMT design ({design.Count} file(s), {plexCount} plex(es)) to:" - + Environment.NewLine + savePath, + + Environment.NewLine + savePath + + (warnings.Any() ? Environment.NewLine + Environment.NewLine + string.Join(Environment.NewLine + Environment.NewLine, warnings) : ""), "Saved"); } diff --git a/MetaMorpheus/TaskLayer/GlycoSearchTask/PostGlycoSearchAnalysisTask.cs b/MetaMorpheus/TaskLayer/GlycoSearchTask/PostGlycoSearchAnalysisTask.cs index 8095fb9d4d..f538f6eed6 100644 --- a/MetaMorpheus/TaskLayer/GlycoSearchTask/PostGlycoSearchAnalysisTask.cs +++ b/MetaMorpheus/TaskLayer/GlycoSearchTask/PostGlycoSearchAnalysisTask.cs @@ -478,9 +478,11 @@ private void QuantificationAnalysis() return; } + // quantified as numbered; said now and in results.txt foreach (string warning in ExperimentalDesign.GetWarningsInExperimentalDesign(spectraFileInfo)) { Warn(warning); + Parameters.GlycoSearchTaskResults?.AddTaskSummaryText(warning); } } else diff --git a/MetaMorpheus/TaskLayer/SearchTask/PostSearchAnalysisTask.cs b/MetaMorpheus/TaskLayer/SearchTask/PostSearchAnalysisTask.cs index 6720409642..b0f7042593 100644 --- a/MetaMorpheus/TaskLayer/SearchTask/PostSearchAnalysisTask.cs +++ b/MetaMorpheus/TaskLayer/SearchTask/PostSearchAnalysisTask.cs @@ -672,6 +672,13 @@ private void MultiplexQuantificationAnalysis() return; } + // quantified as numbered; said now and in results.txt + foreach (string warning in TmtExperimentalDesign.GetWarningsInDesign(tmtFiles)) + { + Warn(warning); + Parameters.SearchTaskResults.AddTaskSummaryText(warning); + } + // Copy the design into the output folder, as the label-free path does for ExperimentalDesign, // so a result folder records the design it was quantified under. AFTER the read, not before: // a design that could not be parsed was not quantified under anything, and archiving it @@ -945,9 +952,11 @@ private void QuantificationAnalysis() return; } + // quantified as numbered; said now and in results.txt foreach (string warning in ExperimentalDesign.GetWarningsInExperimentalDesign(spectraFileInfo)) { Warn(warning); + Parameters.SearchTaskResults.AddTaskSummaryText(warning); } } else if (Parameters.SearchParameters.Normalize) diff --git a/MetaMorpheus/Test/MultiplexQuantificationTests.cs b/MetaMorpheus/Test/MultiplexQuantificationTests.cs index 51cab7c6ec..28d86c63c3 100644 --- a/MetaMorpheus/Test/MultiplexQuantificationTests.cs +++ b/MetaMorpheus/Test/MultiplexQuantificationTests.cs @@ -1227,5 +1227,84 @@ private static Ms2ScanWithSpecificMass ReporterIonScan(string rawPath, int scanN #endregion + #region Numbering gaps + + /// + /// Two files of one plex as fractions 1 and 3, and as fractions 1 and 2. The gapped design used to + /// be refusable only in the GUI (the parser never checked); it is quantified, warned about in the + /// log and in results.txt, and every quantification table is the same as for the contiguous + /// numbering: isobaric quantification does not read the fraction number. + /// + [Test] + public static void TmtFractionGap_IsQuantifiedAsNumberedAndWarned() + { + var gapped = RunTwoFractionTmtSearch("TmtFractionGapGapped", secondFraction: 3); + var contiguous = RunTwoFractionTmtSearch("TmtFractionGapContiguous", secondFraction: 2); + + string expected = "Plex Plex1: fractions 1, 3, quantified as numbered; fraction 2 is not in the design."; + Assert.That(gapped.Warnings, Has.Some.StartWith(expected)); + Assert.That(gapped.ResultsTxt, Does.Contain(expected), "the warning is recorded in results.txt too"); + Assert.That(contiguous.Warnings, Has.None.Contain("not in the design")); + + foreach (string table in new[] { QuantificationWriter.PeptideFileName, QuantificationWriter.ProteinGroupFileName }) + { + Assert.That(gapped.Tables[table], Has.Length.GreaterThan(1), table + " must quantify something"); + Assert.That(gapped.Tables[table], Is.EqualTo(contiguous.Tables[table]), table); + } + } + + private static (List Warnings, string ResultsTxt, Dictionary Tables) RunTwoFractionTmtSearch(string rootName, int secondFraction) + { + string root = Path.Combine(TestContext.CurrentContext.TestDirectory, rootName); + string dataFolder = Path.Combine(root, "data"); + string outputFolder = Path.Combine(root, "out"); + if (Directory.Exists(root)) Directory.Delete(root, true); + Directory.CreateDirectory(dataFolder); + + var warnings = new List(); + EventHandler handler = (o, e) => warnings.Add(e.S); + MetaMorpheusTask.WarnHandler += handler; + try + { + string source = Path.Combine(TestContext.CurrentContext.TestDirectory, @"TMT_test\VA084TQ_6.mzML"); + var fractions = new[] { ("TmtFracA", 1), ("TmtFracB", secondFraction) }; + var stagedPaths = fractions.Select(f => + { + string staged = Path.Combine(dataFolder, f.Item1 + ".mzML"); + File.Copy(source, staged); + return staged; + }).ToList(); + + // One plex, two fractions: the fractions repeat the plex's channel-to-sample map. + var rows = fractions.SelectMany((f, file) => Tmt11Channels.Select((tag, i) => + $"{stagedPaths[file]}\tPlex1\tSample{i + 1}\t{tag}\tCond{(i % 2 == 0 ? "A" : "B")}\t{i / 2 + 1}\t{f.Item2}\t1\tstudy sample")); + File.WriteAllLines( + Path.Combine(dataFolder, GlobalVariables.TmtExperimentalDesignFileName), + new[] { TmtExperimentalDesign.Header }.Concat(rows)); + + var searchTask = Toml.ReadFile( + Path.Combine(TestContext.CurrentContext.TestDirectory, @"TMT_test\TMT-Task1-SearchTaskconfig.toml"), + MetaMorpheusTask.tomlConfig); + searchTask.SearchParameters.DoParsimony = true; + + new EverythingRunnerEngine( + new List<(string, MetaMorpheusTask)> { ("search", searchTask) }, + stagedPaths, + new List { new DbForTask(Path.Combine(TestContext.CurrentContext.TestDirectory, @"TMT_test\mouseTmt.fasta"), false) }, + outputFolder).Run(); + + string searchOut = Path.Combine(outputFolder, "search"); + var tables = new[] { QuantificationWriter.PeptideFileName, QuantificationWriter.ProteinGroupFileName } + .ToDictionary(t => t, t => File.ReadAllLines(Path.Combine(searchOut, t))); + return (warnings, File.ReadAllText(Path.Combine(searchOut, "results.txt")), tables); + } + finally + { + MetaMorpheusTask.WarnHandler -= handler; + if (Directory.Exists(root)) Directory.Delete(root, true); + } + } + + #endregion } } diff --git a/MetaMorpheus/Test/QuantificationTest.cs b/MetaMorpheus/Test/QuantificationTest.cs index eb5ec9c368..16fdae77de 100644 --- a/MetaMorpheus/Test/QuantificationTest.cs +++ b/MetaMorpheus/Test/QuantificationTest.cs @@ -69,7 +69,7 @@ public static void TestExperimentalDesignErrors() Assert.That(readIn.Select(p => p.BiologicalReplicate), Is.EquivalentTo(new[] { 0, 1, 3 })); Assert.That(ExperimentalDesign.GetWarningsInExperimentalDesign(readIn), Has.Count.EqualTo(1)); - // test non-consecutive fractions (should produce an error) + // test non-consecutive fractions, here 2, 3, 4 (a warning, not an error) spectraFiles.Clear(); spectraFiles.Add(new SpectraFileInfo(Path.Combine(outputFolder, @"myFile.1.raw"), "condition1", 0, 0, 1)); @@ -83,9 +83,10 @@ public static void TestExperimentalDesignErrors() spectraFiles.Select(p => p.FullFilePathWithExtension).ToList(), out errors); - Assert.That(errors.Any()); + Assert.That(errors, Is.Empty); + Assert.That(ExperimentalDesign.GetWarningsInExperimentalDesign(readIn), Has.Count.EqualTo(1)); - // test non-consecutive techreps (should produce an error) + // test non-consecutive techreps, here 1, 3, 4 (a warning, not an error) spectraFiles.Clear(); spectraFiles.Add(new SpectraFileInfo(Path.Combine(outputFolder, @"myFile.1.raw"), "condition1", 0, 0, 0)); @@ -99,7 +100,8 @@ public static void TestExperimentalDesignErrors() spectraFiles.Select(p => p.FullFilePathWithExtension).ToList(), out errors); - Assert.That(errors.Any()); + Assert.That(errors, Is.Empty); + Assert.That(ExperimentalDesign.GetWarningsInExperimentalDesign(readIn), Has.Count.EqualTo(1)); // test duplicates (should produce an error) spectraFiles.Clear(); @@ -168,8 +170,8 @@ static SpectraFileInfo Info(string name, string condition, int oneBasedBiorep, i Assert.That(ExperimentalDesign.GetErrorsInExperimentalDesign(gap), Is.Null); Assert.That(ExperimentalDesign.GetWarningsInExperimentalDesign(gap), Is.EqualTo(new[] { - "Condition \"A\" has biorep 1, 2, 4 but not biorep 3. The bioreps are quantified as numbered; " + - "a missing number may be a sample that was lost or not searched." + "Condition \"A\": biological replicates 1, 2, 4, quantified as numbered; biological replicate 3 is not in the design. " + + "A missing number may be a sample or file that was lost or not searched." })); // study-wide numbering: each condition is judged on its own numbers @@ -177,27 +179,119 @@ static SpectraFileInfo Info(string name, string condition, int oneBasedBiorep, i Assert.That(ExperimentalDesign.GetErrorsInExperimentalDesign(studyWide), Is.Null); var warnings = ExperimentalDesign.GetWarningsInExperimentalDesign(studyWide); Assert.That(warnings, Has.Count.EqualTo(1)); - Assert.That(warnings[0], Does.StartWith("Condition \"treated\" has biorep 3, 4 but not biorep 1, 2.")); + Assert.That(warnings[0], Does.StartWith("Condition \"treated\": biological replicates 3, 4, quantified as numbered; biological replicates 1, 2 are not in the design.")); // 1..N says nothing var complete = new List { Info("a1", "A", 1), Info("a2", "A", 2) }; Assert.That(ExperimentalDesign.GetWarningsInExperimentalDesign(complete), Is.Empty); - // a biorep after a gap still has its fractions checked - var fractionGap = new List { Info("a1", "A", 1), Info("a4f2", "A", 4, oneBasedFraction: 2) }; - Assert.That(ExperimentalDesign.GetErrorsInExperimentalDesign(fractionGap), Is.EqualTo("Condition \"A\" biorep 4 fraction 1 is missing!")); + // fractions and techreps: a gap is a warning too, named down to the biorep and fraction + var fractionGap = new List { Info("a1f1", "A", 1), Info("a1f3", "A", 1, oneBasedFraction: 3) }; + Assert.That(ExperimentalDesign.GetErrorsInExperimentalDesign(fractionGap), Is.Null); + Assert.That(ExperimentalDesign.GetWarningsInExperimentalDesign(fractionGap), Is.EqualTo(new[] + { + "Condition \"A\" biorep 1: fractions 1, 3, quantified as numbered; fraction 2 is not in the design. " + + "A missing number may be a sample or file that was lost or not searched." + })); + var techrepGap = new List { new("t2.raw", "A", 0, 1, 0) }; + Assert.That(ExperimentalDesign.GetErrorsInExperimentalDesign(techrepGap), Is.Null); + Assert.That(ExperimentalDesign.GetWarningsInExperimentalDesign(techrepGap), + Has.One.StartWith("Condition \"A\" biorep 1 fraction 1: technical replicates 2, quantified as numbered; technical replicate 1 is not in the design.")); + + // a duplicate is still refused + var duplicate = new List { Info("x", "A", 4), Info("y", "A", 4) }; + Assert.That(ExperimentalDesign.GetErrorsInExperimentalDesign(duplicate), + Is.EqualTo("Duplicates are not allowed:\nCondition \"A\" biorep 4 fraction 1 techrep 1")); } /// /// End to end, with normalization on: a design numbered study-wide (A: 2, 3; B: 4, 5) used to make /// the search skip quantification with one warning. It is now quantified under the numbers given, - /// and the gaps are warned about. The first condition has no biorep 1, which is the normalization - /// reference before mzLib #1422; without #1422 biorep normalization silently does nothing here. + /// and the gaps are warned about, in the log and in results.txt. The first condition has no + /// biorep 1, which is the normalization reference before mzLib #1422; without #1422 biorep + /// normalization silently does nothing here. /// [Test] public static void TestSearchQuantifiesADesignWithABiorepGap() { - string unitTestFolder = Path.Combine(TestContext.CurrentContext.TestDirectory, @"TestSearchQuantifiesADesignWithABiorepGap"); + var run = RunOnePeptideSearch("TestSearchQuantifiesADesignWithABiorepGap", new[] + { + ("A2", "A", 2, 1, 1, 2e6), ("A3", "A", 3, 1, 1, 3e6), ("B4", "B", 4, 1, 1, 4e6), ("B5", "B", 5, 1, 1, 5e6) + }); + + Assert.That(run.Warnings, Has.None.Contain("Skipping quantification")); + foreach (string expected in new[] + { + "Condition \"A\": biological replicates 2, 3, quantified as numbered; biological replicate 1 is not in the design.", + "Condition \"B\": biological replicates 4, 5, quantified as numbered; biological replicates 1, 2, 3 are not in the design." + }) + { + Assert.That(run.Warnings, Has.Some.StartWith(expected)); + Assert.That(run.ResultsTxt, Does.Contain(expected), "the warning is recorded in results.txt too"); + } + + // the protein table's sample columns carry the numbers the design gives + Assert.That(run.ProteinHeader.Where(h => h.StartsWith("Intensity_")), + Is.EquivalentTo(new[] { "Intensity_A_2", "Intensity_A_3", "Intensity_B_4", "Intensity_B_5" })); + + // and every file's peptide was quantified and normalized: the files were written at 2, 3, 4 + // and 5 times one intensity, and normalization brings this one peptide back to one value + var intensities = run.PeptideIntensityByFile.Values.ToList(); + Assert.That(intensities, Has.Count.EqualTo(4)); + Assert.That(intensities, Has.All.GreaterThan(0)); + Assert.That(intensities.Max() / intensities.Min(), Is.LessThan(1.01)); + } + + /// + /// End to end, with normalization on: the same five files numbered with a gap in fractions + /// (1, 3) and in techreps (1, 3), and numbered without one. Both are quantified, the gapped one + /// with warnings, and every file's normalized peptide intensity is the same in both: a gap + /// changes nothing but the numbers. + /// + [Test] + public static void TestSearchQuantifiesFractionAndTechrepGapsAsNumbered() + { + var gapped = RunOnePeptideSearch("TestFractionTechrepGapsGapped", new[] + { + ("Af1t1", "A", 1, 1, 1, 1e6), ("Af1tB", "A", 1, 1, 3, 2e6), ("AfBt1", "A", 1, 3, 1, 3e6), + ("Bf1t1", "B", 1, 1, 1, 4e6), ("BfBt1", "B", 1, 3, 1, 5e6) + }); + var contiguous = RunOnePeptideSearch("TestFractionTechrepGapsContiguous", new[] + { + ("Af1t1", "A", 1, 1, 1, 1e6), ("Af1tB", "A", 1, 1, 2, 2e6), ("AfBt1", "A", 1, 2, 1, 3e6), + ("Bf1t1", "B", 1, 1, 1, 4e6), ("BfBt1", "B", 1, 2, 1, 5e6) + }); + + Assert.That(gapped.Warnings, Has.None.Contain("Skipping quantification")); + foreach (string expected in new[] + { + "Condition \"A\" biorep 1: fractions 1, 3, quantified as numbered; fraction 2 is not in the design.", + "Condition \"A\" biorep 1 fraction 1: technical replicates 1, 3, quantified as numbered; technical replicate 2 is not in the design.", + "Condition \"B\" biorep 1: fractions 1, 3, quantified as numbered; fraction 2 is not in the design." + }) + { + Assert.That(gapped.Warnings, Has.Some.StartWith(expected)); + Assert.That(gapped.ResultsTxt, Does.Contain(expected)); + } + Assert.That(contiguous.Warnings, Has.None.Contain("not in the design")); + + Assert.That(gapped.PeptideIntensityByFile.Keys, Is.EquivalentTo(contiguous.PeptideIntensityByFile.Keys)); + Assert.That(gapped.PeptideIntensityByFile.Values, Has.All.GreaterThan(0)); + foreach (var (file, intensity) in contiguous.PeptideIntensityByFile) + { + Assert.That(gapped.PeptideIntensityByFile[file], Is.EqualTo(intensity).Within(1e-9).Percent, file); + } + } + + /// + /// Runs a search with normalization on over one-peptide files written at the given intensities, + /// under a design with the given one-based numbers, and returns what it warned, results.txt, the + /// protein table's header and the peptide table's intensity per file. + /// + private static (List Warnings, string ResultsTxt, string[] ProteinHeader, Dictionary PeptideIntensityByFile) + RunOnePeptideSearch(string folderName, (string Name, string Condition, int Biorep, int Fraction, int Techrep, double Intensity)[] files) + { + string unitTestFolder = Path.Combine(TestContext.CurrentContext.TestDirectory, folderName); if (Directory.Exists(unitTestFolder)) { Directory.Delete(unitTestFolder, true); @@ -205,16 +299,15 @@ public static void TestSearchQuantifiesADesignWithABiorepGap() _ = Directory.CreateDirectory(unitTestFolder); string peptide = "PEPTIDE"; - Protein prot = new(peptide, @"test"); string dbName = Path.Combine(unitTestFolder, "testDB.fasta"); - UsefulProteomicsDatabases.ProteinDbWriter.WriteFastaDatabase(new List { prot }, dbName, ">"); + UsefulProteomicsDatabases.ProteinDbWriter.WriteFastaDatabase(new List { new(peptide, @"test") }, dbName, ">"); var fileInfos = new List(); - foreach (var (condition, oneBasedBiorep) in new[] { ("A", 2), ("A", 3), ("B", 4), ("B", 5) }) + foreach (var f in files) { - string fullPath = Path.Combine(unitTestFolder, condition + oneBasedBiorep + ".mzML"); - WriteOnePeptideMzml(peptide, 1e6 * oneBasedBiorep, fullPath); - fileInfos.Add(new SpectraFileInfo(fullPath, condition, oneBasedBiorep - 1, 0, 0)); + string fullPath = Path.Combine(unitTestFolder, f.Name + ".mzML"); + WriteOnePeptideMzml(peptide, f.Intensity, fullPath); + fileInfos.Add(new SpectraFileInfo(fullPath, f.Condition, f.Biorep - 1, f.Techrep - 1, f.Fraction - 1)); } _ = ExperimentalDesign.WriteExperimentalDesignToFile(fileInfos); @@ -232,28 +325,16 @@ public static void TestSearchQuantifiesADesignWithABiorepGap() MetaMorpheusTask.WarnHandler -= handler; } - Assert.That(warnings, Has.None.Contain("Skipping quantification")); - Assert.That(warnings, Has.Some.StartWith("Condition \"A\" has biorep 2, 3 but not biorep 1.")); - Assert.That(warnings, Has.Some.StartWith("Condition \"B\" has biorep 4, 5 but not biorep 1, 2, 3.")); - - // the protein table's sample columns carry the numbers the design gives - string proteinTable = Path.Combine(unitTestFolder, "AllQuantifiedProteinGroups.tsv"); - Assert.That(File.Exists(proteinTable)); - var proteinHeader = File.ReadAllLines(proteinTable)[0].Split('\t'); - Assert.That(proteinHeader.Where(h => h.StartsWith("Intensity_")), - Is.EquivalentTo(new[] { "Intensity_A_2", "Intensity_A_3", "Intensity_B_4", "Intensity_B_5" })); + string resultsTxt = File.ReadAllText(Path.Combine(unitTestFolder, "results.txt")); + string[] proteinHeader = File.ReadAllLines(Path.Combine(unitTestFolder, "AllQuantifiedProteinGroups.tsv"))[0].Split('\t'); - // and every file's peptide was quantified and normalized: the files were written at 2, 3, 4 - // and 5 times one intensity, and normalization brings this one peptide back to one value - string[] lines = File.ReadAllLines(Path.Combine(unitTestFolder, "AllQuantifiedPeptides.tsv")); - var header = lines[0].Split('\t').ToList(); - var intensities = new[] { "Intensity_A2", "Intensity_A3", "Intensity_B4", "Intensity_B5" } - .Select(column => double.Parse(lines[1].Split('\t')[header.IndexOf(column)])) - .ToList(); - Assert.That(intensities, Has.All.GreaterThan(0)); - Assert.That(intensities.Max() / intensities.Min(), Is.LessThan(1.01)); + string[] peptideLines = File.ReadAllLines(Path.Combine(unitTestFolder, "AllQuantifiedPeptides.tsv")); + var peptideHeader = peptideLines[0].Split('\t').ToList(); + var peptideIntensityByFile = files.ToDictionary(f => f.Name, + f => double.Parse(peptideLines[1].Split('\t')[peptideHeader.IndexOf("Intensity_" + f.Name)])); Directory.Delete(unitTestFolder, true); + return (warnings, resultsTxt, proteinHeader, peptideIntensityByFile); } /// diff --git a/MetaMorpheus/Test/TestCmd.cs b/MetaMorpheus/Test/TestCmd.cs index 22b00977a7..cbe0a34aab 100644 --- a/MetaMorpheus/Test/TestCmd.cs +++ b/MetaMorpheus/Test/TestCmd.cs @@ -1316,28 +1316,57 @@ public static void TestSdrfDesignKeepsStudyWideReplicatesAndSaysSo() var console = new List(); Assert.That(Program.ResolveExperimentalDesign(data, searched, normalizationRequested: true, reportToConsole: true, write: console.Add), Is.EqualTo(0)); - Assert.That(console, Has.Some.StartWith("Condition \"FA\" has biorep 3, 4 but not biorep 1, 2.")); + Assert.That(console, Has.Some.StartWith("Condition \"FA\": biological replicates 3, 4, quantified as numbered; biological replicates 1, 2 are not in the design.")); } /// /// A design MetaMorpheus would reject is never written, because a run finding it would skip - /// quantification with only a warning. Exit 5, every reason printed. + /// quantification with only a warning. Exit 5, every reason printed. The one such design is a + /// duplicate: two files at one condition, biorep, fraction and techrep. /// [Test] - public static void TestSdrfDesignRefusesAFractionGapAndWritesNothing() + public static void TestSdrfDesignRefusesADuplicateAndWritesNothing() { var (sdrf, data) = WriteLabelFreeSdrf( - ("a1.raw", "WT", "DMSO", 1, 1), ("a3.raw", "WT", "DMSO", 1, 3)); + ("a1.raw", "WT", "DMSO", 1, 1), ("a2.raw", "WT", "DMSO", 1, 1)); var settings = SdrfDesignSettings(sdrf, data, "genotype"); var written = new StringWriter(); int exitCode = Program.WriteDesignFromSdrf(settings, written); Assert.That(exitCode, Is.EqualTo(5)); - Assert.That(written.ToString(), Does.Contain("fraction 2 is missing").And.Contain("No design file was written")); + Assert.That(written.ToString(), Does.Contain("biorep 1 fraction 1 techrep 1 is named by 2 files").And.Contain("No design file was written")); Assert.That(File.Exists(Path.Combine(data, GlobalVariables.ExperimentalDesignFileName)), Is.False); } + /// + /// A fraction gap is written as the SDRF numbers it, and the SDRF report and the run say so in the + /// same words: mzLib notes "fractions 1, 3, kept as the SDRF numbers them", and the run's design check + /// warns "fractions 1, 3, quantified as numbered". + /// + [Test] + public static void TestSdrfDesignWritesAFractionGapAndBothSidesSaySo() + { + var (sdrf, data) = WriteLabelFreeSdrf( + ("a1.raw", "WT", "DMSO", 1, 1), ("a3.raw", "WT", "DMSO", 1, 3)); + var settings = SdrfDesignSettings(sdrf, data, "genotype"); + + var written = new StringWriter(); + Assert.That(Program.WriteDesignFromSdrf(settings, written), Is.EqualTo(0), written.ToString()); + Assert.That(written.ToString(), Does.Contain("Condition 'WT' biorep 1: fractions 1, 3, kept as the SDRF numbers them.")); + + var searched = settings.Spectra.Select(Path.GetFullPath).ToList(); + var readBack = ExperimentalDesign.ReadExperimentalDesign( + Path.Combine(data, GlobalVariables.ExperimentalDesignFileName), searched, out var errors); + Assert.That(errors, Is.Empty); + Assert.That(readBack.Select(f => f.Fraction), Is.EquivalentTo(new[] { 0, 2 })); + + var console = new List(); + Assert.That(Program.ResolveExperimentalDesign(data, searched, normalizationRequested: true, reportToConsole: true, write: console.Add), + Is.EqualTo(0)); + Assert.That(console, Has.Some.StartWith("Condition \"WT\" biorep 1: fractions 1, 3, quantified as numbered; fraction 2 is not in the design.")); + } + [Test] public static void TestSdrfDesignRefusesASpectraFileWithNoRow() { diff --git a/MetaMorpheus/Test/TestOGlyco.cs b/MetaMorpheus/Test/TestOGlyco.cs index e4e95c5ee6..8a0dca5d75 100644 --- a/MetaMorpheus/Test/TestOGlyco.cs +++ b/MetaMorpheus/Test/TestOGlyco.cs @@ -958,9 +958,10 @@ public static void GlycoTestWithBadExperimentalDesignFile() List spectraFiles = new List(); - //These conditions are such that the experimental design file is bad. - spectraFiles.Add(new SpectraFileInfo(spectraFile1, "condition1", 0, 9, 0)); - spectraFiles.Add(new SpectraFileInfo(spectraFile2, "condition2", 5, 0, 4)); + //These conditions are such that the experimental design file is bad: two files at one + //condition, biorep, fraction and techrep. (A gap in the numbers is only a warning.) + spectraFiles.Add(new SpectraFileInfo(spectraFile1, "condition1", 0, 0, 0)); + spectraFiles.Add(new SpectraFileInfo(spectraFile2, "condition1", 0, 0, 0)); ExperimentalDesign.WriteExperimentalDesignToFile(spectraFiles); @@ -973,7 +974,7 @@ public static void GlycoTestWithBadExperimentalDesignFile() out var errors); Assert.That(errors.Count == 1); - Assert.That(errors[0].Contains("Condition \"condition1\" biorep 1 fraction 1 techrep 1 is missing!")); + Assert.That(errors[0], Is.EqualTo("Duplicates are not allowed:\nCondition \"condition1\" biorep 1 fraction 1 techrep 1")); Assert.That(readIn.Count == 2); new EverythingRunnerEngine(new List<(string, MetaMorpheusTask)> { ("Task", glycoSearchTask) }, new List { spectraFile1, spectraFile2 }, new List { db }, outputFolder).Run(); diff --git a/MetaMorpheus/Test/TmtExperimentalDesignTests.cs b/MetaMorpheus/Test/TmtExperimentalDesignTests.cs index f0d8242c18..24134ebe21 100644 --- a/MetaMorpheus/Test/TmtExperimentalDesignTests.cs +++ b/MetaMorpheus/Test/TmtExperimentalDesignTests.cs @@ -804,5 +804,56 @@ public static void APlaceholderRowForAnUnannotatedFileSurvivesARoundTrip() #endregion + #region Design rules lifted from the window (#2793) + + private static TmtFileInfo PlexFile(string name, string plex, int fraction, int techrep) => + new TmtFileInfo(name + ".raw", plex, fraction, techrep, null); + + /// + /// The window's refusals, now in EngineLayer: a file with no plex, a number below 1, and two + /// files at one (fraction, technical replicate) of a plex. Two plexes may each have their own + /// (1, 1). + /// + [Test] + public static void TmtDesignErrorsAreTheWindowRules() + { + Assert.That(TmtExperimentalDesign.GetErrorsInDesign(new[] { PlexFile("a", "", 1, 1) }), + Is.EqualTo("Every file must be assigned a Plex. Missing for: a.raw")); + Assert.That(TmtExperimentalDesign.GetErrorsInDesign(new[] { PlexFile("a", "P1", 1, 0) }), + Is.EqualTo("Technical Replicate values must be >= 1.")); + Assert.That(TmtExperimentalDesign.GetErrorsInDesign(new[] { PlexFile("a", "P1", 0, 1) }), + Is.EqualTo("Fraction values must be >= 1.")); + Assert.That(TmtExperimentalDesign.GetErrorsInDesign(new[] { PlexFile("a", "P1", 2, 1), PlexFile("b", "p1", 2, 1) }), + Is.EqualTo("Plex P1: duplicate Fraction/Technical Replicate combination: Fraction 2, Technical Replicate 1.")); + Assert.That(TmtExperimentalDesign.GetErrorsInDesign(new[] { PlexFile("a", "P1", 1, 1), PlexFile("b", "P2", 1, 1) }), + Is.Null); + } + + /// + /// A gap in a plex's fractions, or in a fraction's technical replicates, was refused by the window + /// and is now a warning naming the missing numbers. Each plex is judged on its own numbers. + /// + [Test] + public static void TmtDesignGapsAreWarningsNotErrors() + { + var gapped = new[] + { + PlexFile("a", "P1", 1, 1), PlexFile("b", "P1", 3, 1), PlexFile("c", "P1", 3, 3), + PlexFile("d", "P2", 2, 1) + }; + + Assert.That(TmtExperimentalDesign.GetErrorsInDesign(gapped), Is.Null); + Assert.That(TmtExperimentalDesign.GetWarningsInDesign(gapped), Is.EqualTo(new[] + { + "Plex P1: fractions 1, 3, quantified as numbered; fraction 2 is not in the design. A missing number may be a file that was lost or not searched.", + "Plex P1 fraction 3: technical replicates 1, 3, quantified as numbered; technical replicate 2 is not in the design. A missing number may be a file that was lost or not searched.", + "Plex P2: fractions 2, quantified as numbered; fraction 1 is not in the design. A missing number may be a file that was lost or not searched." + })); + + var contiguous = new[] { PlexFile("a", "P1", 1, 1), PlexFile("b", "P1", 2, 1), PlexFile("c", "P1", 2, 2) }; + Assert.That(TmtExperimentalDesign.GetWarningsInDesign(contiguous), Is.Empty); + } + + #endregion } }