fix(pride config): scale ISOBARIC_WORKFLOW and MSSTATS_CONVERTER with experiment size - #718
Merged
Merged
Conversation
… experiment size
Both are SINGLE processes that aggregate the whole experiment, so per-file defaults do
not fit large runs - the same reason PROTEOMICSLFQ already scales here. Measured on
MSV000085836 (552 TMT fractions, pride_codon_slurm):
ISOBARIC_WORKFLOW 72GB default -> OOM (exit 137) after 2h36m, succeeded only via the
retry escalation, peak RSS 118.7GB. Hours wasted on a first attempt
that could never fit.
MSSTATS_CONVERTER 12GB default -> OOM-killed 9 times in a row. The retry ladder never
reaches a workable size, so the run dies after exhausting attempts.
ISOBARIC_WORKFLOW takes the same path(mzmls) input as PROTEOMICSLFQ, so it reuses the
identical file-count rule. MSSTATS_CONVERTER receives the single merged consensusXML
instead of a file list, so it scales on that file's size.
.size() means BYTES for a single Path but COUNT for a List, so the consensusXML closure
guards the List case explicitly - a future change to a collected channel would otherwise
silently take the small branch and reintroduce the OOM.
Contributor
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the pride_codon_slurm execution profile to avoid out-of-memory failures on large PRIDE reanalysis runs by scaling resource requests for experiment-aggregating processes based on experiment size (file count or merged file size), instead of using per-file defaults.
Changes:
- Add experiment-size-based memory/CPU scaling for
ISOBARIC_WORKFLOWusing the same file-count rule already used forPROTEOMICSLFQ. - Add input-file-size-based memory scaling for
MSSTATS_CONVERTER, with an explicit guard for the “List vs single Path”.size()behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
daichengxin
approved these changes
Jul 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ISOBARIC_WORKFLOWandMSSTATS_CONVERTERare single processes that aggregate the whole experiment — likePROTEOMICSLFQ, which this config already scales. Their per-file defaults do not fit large runs, and both failed on a real 552-fraction TMT dataset (MSV000085836) underpride_codon_slurm:ISOBARIC_WORKFLOWMSSTATS_CONVERTERMSSTATS_CONVERTERis the worse of the two: starting at 12 GB, the* task.attemptladder never reaches a workable size for an experiment this big, so every attempt dies and the pipeline fails after ~17 hours of otherwise-successful work (conversion, both search engines, ms2features, Percolator and protein inference had all completed).Fix
Scale both with the experiment, reusing the pattern already established here for
PROTEOMICSLFQ/ASSEMBLE_EMPIRICAL_LIBRARY:ISOBARIC_WORKFLOWtakes the samepath(mzmls)input asPROTEOMICSLFQ, so it reuses the identical file-count rule (< 200 files ? 72 GB : 250 GB, ×task.attempt).MSSTATS_CONVERTERreceives the single mergedconsensusXMLrather than a file list, so it scales on that file's size instead (< 2 GB ? 24 GB : 200 GB, ×task.attempt).One subtlety worth reviewing
.size()means bytes for a singlePathbut element count for aList.consensusXMLis currently a single file (tmt.nf:61passesISOBARIC_WORKFLOW.out.out_consensusXML), so the byte reading is correct today — but the closure guards theListcase explicitly, because a future change to a collected channel would otherwise silently take the small branch and quietly reintroduce exactly this OOM.Notes
resourceLimits(1900 GB) and fit standard ~480 GB nodes, so nobigmemrouting is needed.