Skip to content

Remove executor dependency from accelerator directive#2181

Open
pditommaso wants to merge 1 commit into
devfrom
fix-accelerator-portability
Open

Remove executor dependency from accelerator directive#2181
pditommaso wants to merge 1 commit into
devfrom
fix-accelerator-portability

Conversation

@pditommaso
Copy link
Copy Markdown

@pditommaso pditommaso commented May 7, 2026

Summary

  • The accelerator directive for PARABRICKS_FQ2BAM is currently gated on a hardcoded list of executors (awsbatch, google-batch, hq, k8s). This breaks the portability contract of Nextflow pipelines: the same process should run consistently across executors, and GPU resource requirements are a property of the task, not of where it runs.
  • If an executor (e.g. SLURM, LSF, a future cloud batch backend) supports GPUs but is not in the allowlist, accelerator is silently set to null and the GPU request is dropped — leading to either failures or jobs running on CPU-only nodes without warning.
  • This PR makes accelerator an unconditional value in conf/base.config, conf/test.config, and the example in docs/usage.md. Users who need executor-specific behavior can still override it in a custom config, which is the standard Nextflow extension point.

Note for reviewers

docs/usage.md:185 still describes the old executor-conditional behavior in prose ("The accelerator directive is added automatically for the executors ..."). That paragraph is now stale and should be rewritten in a follow-up (or here if preferred).

Test plan

  • CI passes on dev
  • PARABRICKS_FQ2BAM runs on a non-allowlisted executor with GPUs and picks up the directive
  • Existing test profile (test.config) still launches correctly with accelerator = 1

🤖 Generated with Claude Code

@pditommaso @claude
Remove executor dependency from accelerator directive
dc43b8f 
The accelerator directive should not be conditioned on task.executor.
Pipelines must remain portable across executors; gating GPU resource
requests on a hardcoded executor allowlist breaks that contract.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@nf-core-bot
Copy link
Copy Markdown
Member

nf-core-bot commented May 7, 2026

Warning

Newer version of the nf-core template is available.

Your pipeline is using an old version of the nf-core template: 3.5.1.
Please update your pipeline to the latest version.

For more documentation on how to update your pipeline, please see the nf-core documentation and Synchronisation documentation.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 7, 2026 •
edited
Loading

nf-core pipelines lint overall result: Passed ✅ ⚠️

Posted for pipeline commit dc43b8f

+| ✅ 224 tests passed       |+
#| ❔  13 tests were ignored |#
!| ❗   7 tests had warnings |!
Details

❗ Test warnings:

  • pipeline_todos - TODO string in main.nf: Optionally add in-text citation tools to this list.
  • pipeline_todos - TODO string in main.nf: Optionally add bibliographic entries to this list.
  • pipeline_todos - TODO string in main.nf: Only uncomment below if logic in toolCitationText/toolBibliographyText has been filled!
  • pipeline_todos - TODO string in base.config: Check the defaults for all processes
  • schema_lint - Input mimetype is missing or empty
  • schema_description - No description provided in schema for parameter: markduplicates_pixel_distance
  • schema_description - No description provided in schema for parameter: gatk_pcr_indel_model

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 3.5.1
  • Run at 2026-05-07 12:14:08

@FriederikeHanssen
Copy link
Copy Markdown
Contributor

FriederikeHanssen commented May 7, 2026

@famosab can you take a look when you have time. IIRC this was added after some issues on the M3 cluster

@famosab
Copy link
Copy Markdown
Contributor

famosab commented May 7, 2026

Reasoning is if a cluster uses a dedicated gpu queue setting the accelerator to anything makes not sense, that is why it was added like this. I am fine with removing it here and just having that in my cluster config but I thought it should be written down and set to a sensible default somewhere.

You will need to add the accelerator directive to your local config in order to make use of the GPUs correctly if you do not use a cluster setup with a dedicated GPU queue. The accelerator diretive is added automatically for the executors 'awsbatch','google-batch','hq','k8s'. See Nextflow executors for more infos.

@famosab
Copy link
Copy Markdown
Contributor

famosab commented May 7, 2026

I believe there must be a better way than just setting it to 4 here, it would want to at least easily support gpu queues in clusters that have these without the user needing to set it up. But I can see that hardcoding a list of executors does not make sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@FriederikeHanssen FriederikeHanssen Awaiting requested review from FriederikeHanssen FriederikeHanssen is a code owner

@maxulysse maxulysse Awaiting requested review from maxulysse maxulysse is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@pditommaso @nf-core-bot @FriederikeHanssen @famosab