Skip to content

fix(trimgalore): stack process_low_memory on top of process_low#11541

Merged
pinin4fjords merged 4 commits intonf-core:masterfrom
pinin4fjords:pinin4fjords/trimgalore-low-memory
May 7, 2026
Merged

fix(trimgalore): stack process_low_memory on top of process_low#11541
pinin4fjords merged 4 commits intonf-core:masterfrom
pinin4fjords:pinin4fjords/trimgalore-low-memory

Conversation

@pinin4fjords
Copy link
Copy Markdown
Member

@pinin4fjords pinin4fjords commented May 6, 2026

Summary

Replaces label 'process_low' with process_medium + process_low_memory (the latter shipped in nf-core/tools#4264).

label 'process_medium'
label 'process_low_memory'

Effective first-attempt budget: 6 cpus / 1 GB / 8 h.

Why

#11531 couldn't drop memory below 12 GB without also losing the cpu band; process_low_memory now lets us pick the axes independently. process_medium is the first cpu band where trim_galore actually gets a second worker thread (cores = max(1, task.cpus - 4) paired → 2 workers at cpus=6, still 1 at cpus≤5). Memory drops to 1 GB, ~10× the observed ~100 MB peak on 30M PE.

Downstream

Requires the consuming pipeline's base.config to define process_low_memory (synced from a tools release including #4264). Otherwise memory falls through to the process default; the module still works.

Now that nf-core/tools#4264 has merged and the pipeline-template
`base.config` defines `process_low_memory` (1 GB * task.attempt),
the trimgalore module can drop its memory ceiling without giving up
the cpu/time bands set by process_low.

Composed budget on first attempt:
- cpus    = 2     (process_low; 1 worker thread paired)
- memory  = 1 GB  (process_low_memory; ~10x observed peak_rss of ~100 MB on 30M PE)
- time    = 4 h   (process_low; ~160x observed runtime of ~1.5 min on 30M PE)

The tool itself caps trim_galore --cores at 8 worker threads, so
allocating more cpus is wasted unless paired with very large inputs;
pipelines that genuinely need that throughput can override per-process
or stack `process_high` instead. This is the conservative default
that satisfies Felix's original 100 MB / 10 min ask while keeping
existing 1-worker-paired throughput intact for typical use.
The template's process default is cpus=1 / time=4.h, which is what
process_low_memory already inherits when no other resource label is
applied. process_low's cpus=2 changes nothing useful here:
trim_galore's --cores formula (`cores = max(1, task.cpus - 4)` paired)
saturates at 1 worker thread for any cpus <= 5, so 1 vs 2 cpus gives
the same worker count. Drop the redundant label.
Stack process_medium with process_low_memory so the cpu allocation
is enough to give trim_galore 2 paired worker threads on production
data, while keeping the memory ceiling tight at 1 GB.

trim_galore's --cores formula derives worker count from task.cpus:
  cores = max(1, task.cpus - 4)  // paired
  cores = max(1, task.cpus - 3)  // single (capped at 8)

Implications:
- cpus = 1 (default)        -> 1 worker thread paired
- cpus = 2 (process_low)    -> 1 worker thread paired (no gain)
- cpus = 6 (process_medium) -> 2 worker threads paired (~2x throughput)
- cpus = 12 (process_high)  -> 8 worker threads paired (saturates the cap)

process_medium is the cheapest band that crosses the 1->2 worker
threshold, where actual trimming throughput first improves. Useful for
real production inputs (100M+ PE) where the extra worker materially
shortens wall time; small test data won't notice the difference. Memory
stays at process_low_memory's 1 GB regardless.
@SPPearce
Copy link
Copy Markdown
Contributor

SPPearce commented May 6, 2026

Why not just hardcode it for now. Like the bwa/index module: https://github.com/nf-core/modules/blob/master/modules/nf-core/bwa/index/main.nf#L5

@pinin4fjords
Copy link
Copy Markdown
Member Author

Why not just hardcode it for now. Like the bwa/index module: https://github.com/nf-core/modules/blob/master/modules/nf-core/bwa/index/main.nf#L5

The label is a thing now anyway nf-core/tools#4264, no need to hard code

@pinin4fjords pinin4fjords added this pull request to the merge queue May 7, 2026
Merged via the queue into nf-core:master with commit 0991b10 May 7, 2026
61 checks passed
@pinin4fjords pinin4fjords deleted the pinin4fjords/trimgalore-low-memory branch May 7, 2026 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants