Anyone here implemented GPU stuff in nf-core workf...
# request-review
j
Anyone here implemented GPU stuff in nf-core workflows? Could you have a look at my Parabricks fix here? https://github.com/nf-core/rnaseq/pull/1711
👀 1
m
@Famke Bäuerle any things that seem out of place?
f
I think that might interfere with a cluster config where things will be sent to different queues and also maybe when using a cloud environment. But i am not 100% sure.
m
so if I understand well, you want to only apply the gpu flags to gpu processes while getting rid of the gpu profile?
f
But I can see the problem that you are describing, I am confused though because only the tagged processes should get the gpu flags in my understanding
j
i just didn't see the reason for the profile, but I can put it back if there's rationale
f
also for parabricks the gpu number is set via the accelerator directive right? accelerator = { task.executor in ['awsbatch','google-batch','hq','k8s'] ? 1 : null } Its all a bit confusing we might need a visual representation what settings we put where to ensure that it works with all environments.
I think what you are describing here:
The previous
gpu
profile set
docker.runOptions = '--gpus all'
globally, meaning every Docker container got
--gpus all
, not just GPU tasks. On heterogeneous clusters where CPU tasks run on non-GPU nodes, this caused Docker to fail with
could not select device driver "" with capabilities: [[gpu]]
.
needs to be solved but I would solve this via the process directive:
Copy code
withLabel: process_gpu {
        ext.use_gpu = { workflow.profile.contains('gpu') }
        accelerator = { workflow.profile.contains('gpu') ? 1 : null }
    }
Pinging @Adam Talbot as he commented the same thing essentially.
j
That's exactly what the PR does. The fix moves the container GPU flags into the
process_gpu
label via `containerOptions`:
Copy code
withLabel: process_gpu {
    ext.use_gpu      = true
    accelerator      = 1
    containerOptions = {
        if (workflow.containerEngine in ['singularity', 'apptainer']) {
            return '--nv'
        }
        return '--gpus all'
    }
}
The old
process_gpu
block had
accelerator
and
ext.use_gpu
but was missing the actual container runtime flags. Those were in the
gpu
profile as global `docker.runOptions`/`apptainer.runOptions`/`singularity.runOptions`, which hit every container, not just GPU ones.
containerOptions
is the per-process equivalent of
runOptions
. It passes flags to whatever container engine is active, scoped to just that process. So
--gpus all
only ends up on the
docker run
for Parabricks tasks. On `accelerator`: the Parabricks module reads
task.accelerator
to set
--num-gpus
in the pbrun command, so making it executor-conditional would drop that flag on local/SLURM runs. Setting it unconditionally to
1
is harmless where unsupported (those executors ignore it) and keeps pbrun working everywhere. Re the
gpu
profile: the reason I removed it rather than keeping it is that
process_gpu
tasks only run when
--use_parabricks_star
is set, so the flags are never applied when you're not using GPU features. There's no need for an extra opt-in step. But happy to discuss if there's a use case I'm missing.
f
I can check in later again because I have a meeting now but the one thing that I remember from our sarek adventures is that setting the accelerator lead to to runs breaking on my cluster which is why we only set it for specific executors.
j
OK thanks, I'll do some digging!
But seems like reinstating the profile is the best way to avoid complex conditioning on executors, will just do that
f
Here is what I added to the sarek docs:
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](https://www.nextflow.io/docs/latest/executor.html) for more infos.
@Edmund did some digging back then to optimize for AWS https://github.com/nf-core/sarek/pull/2035 I still agree with your point that we have to make sure that the GPU options are not getting added to all docker containers though
👍 1
j
Does this mean we should change the Parabricks modules like this? https://github.com/nf-core/modules/pull/10118
m
Oh
f
I am not exactly sure if that makes sense, but I need to research more how the cluster options flow into task.accelerator (see https://github.com/nextflow-io/nextflow/issues/997 for the original nf discussion). I think there is a reason why we hand this directly.
j
So, based on that issue and the PR, the modules change is right because
accelerator
was always designed as an executor scheduling directive (telling K8s/AWS/local which GPUs to provision), not a way to pass runtime parameters to tools - and once nextflow-io/nextflow#5850 lands, Nextflow will actively manage
CUDA_VISIBLE_DEVICES
based on
accelerator
, meaning the value the executor assigns may not even match what the tool should receive as
--num-gpus
, making the decoupling to
task.ext.num_gpus
essential.
f
because omitting
accelerator
on local/SLURM also drops
--num-gpus
from the tool command.
I think num-gpus is only needed if we do not specify --gpus all or something cluster specific and it is not a prblem that we drop it from the run command
a
I don't think there's a neat, 1-size-fits-all solution for GPU in docker. There are many distributions of the Nvidia Docker container runtime and many different ways of passing the GPU into the container For example, using
--gpus all
has attached all the GPUs to the container. On a big node, you might have 8 GPUs attached to 8 or more running tasks! You really want
--gpus $num
or indexed with
--gpus '"device=1,2"'
I can't see
--num-gpus
in the help?
j
I trust that @Gary Burnett knew what he was doing on that
a
Tell 'em to keep their documentation up to date shakefist
f
At least for fq2bam
Number of GPUs to use for a run. (default: 1)
so if its dropped its set to 1
a
These days, I have a little computer friend who can read the source code and make my own docs claude deal with it
j
sooo.....
Copy code
containerOptions = {                                                       
      if (workflow.profile.contains('gpu')) {               
          if (workflow.containerEngine in ['singularity', 'apptainer']) {
              return '--nv'
          }
          def ngpus = task.accelerator?.request ?: 1
          return "--gpus ${ngpus}"
      }
      return null
  }
???
f
i think we are confusing the docker options and the pbrun options
a
Won't that be
--gpus null
? in certain situations?
f
--num-gpus is always 1 and can be increased based on the accelerator directive --gpus needs a more finegrained approach that is dependent on the setup of the environment and cannot be solved on the pipeline level
I can't see
--num-gpus
in the help?
because that is a parabricks command
a
ahhh
j
Won't that be
--gpus null
? in certain situations?
No, the safe navigation will get it to 1
👍 1
a
I think y'all should just set the
process.containerOptions
for Parabricks all the time. I'm not sure what benefit having it behind conditionals like "--profile gpus" adds?
f
I would rather think about how we can make sure that only the processes that are gpu labelled get the containerOptions and not tune the containerOptions because they can be set by the user in their config but we need to make sure that gpu options are assigned correctly.
a
Copy code
params.gpu_container_options = "--gpus 1"

withLabel: process_gpu {
    containerOptions = params.gpu_container_options
}
Simple, easy to validate, works everywhere
j
In proteinfold we are modifying them in the
nextflow.config
for each container technology e.g.:
https://github.com/JoseEspinosa/nf-core-proteinfold/blob/8bb88b75fafeb9bceb19e10780aa09cda053a37f/nextflow.config#L286-L287
j
OK, so final solution: Goal: Only
process_gpu
-labelled processes get GPU container flags. Pipeline provides sensible defaults, users can tune via a param or override
containerOptions
in their own config. nf-core/rnaseq#1711:
Copy code
withLabel: process_gpu {
    ext.use_gpu      = true
    accelerator      = 1
    containerOptions = {
        if (workflow.profile.contains('gpu')) {
            if (workflow.containerEngine in ['singularity', 'apptainer']) {
                return '--nv'
            }
            return params.gpu_container_options ?: '--gpus all'
        }
        return null
    }
}
gpu
profile kept as opt-in. Users can tune Docker GPU flags via
--gpu_container_options '--gpus 1'
etc.
accelerator = 1
for executor-level scheduling (K8s, AWS Batch, local once nextflow-io/nextflow#5850 lands). nf-core/modules#10118:
Copy code
// before
def num_gpus = task.accelerator ? "--num-gpus ${task.accelerator.request}" : ''
// after
def num_gpus = task.ext.num_gpus ? "--num-gpus ${task.ext.num_gpus}" : ''
accelerator
is an executor scheduling directive (nextflow-io/nextflow#997), not a tool parameter. Decoupled across all 11 Parabricks modules. Does this look right?
j
Sorry, just realised that the idea is actually to remove them from the
nextflow.config
😅
a
Does this look right?
Why do this?
Copy code
return params.gpu_container_options ?: '--gpus all'
When setting the
params.gpu_container_options
to have a default is easier to control? I'm waging a war against random profiles and secret configs 😆
j
Yep, thanks @Jose Espinosa-Carrasco 🙂
When setting the
params.gpu_container_options
to have a default is easier to control?
fair
Adam's PR comment that a special
gpu
param is probably more useful than a profile sounded sensible to me. PR now updated: https://github.com/nf-core/rnaseq/pull/1711 (see also https://github.com/nf-core/modules/pull/10118)
a
I believe proteinfold uses a simple
--gpus
boolean flag, right @Jose Espinosa-Carrasco?
j
yes, we use
--use-gpu
j
Could someone give me a call on whether https://github.com/nf-core/rnaseq/pull/1711 and the modules PR are correct now? Maybe also @Gary Burnett if you're back today!
👀 1
m
I'm still unsure about removing the gpu profile
a
Why? what does
-profile gpu
add that
--gpu
doesn't? But
--gpu
can be validated, checked. Doesn't require introspection...?
m
I see your point but it's coming from TEMPLATE, so for me, it's a more complex problem. Do we still want it in the TEMPLATE or not? Removing it from this pipeline, would mean that we should remove it in ALL pipelines
j
the fact it wasn't working well here suggests it hasn't been widely battle tested elsewhere... I don't have a problem with rnaseq being a bit different while this gets figured out
m
Fine by me then
As long as we agree and go forth with this, as we have at least a couple of flagship pipelines using gpus
a
I don't think there is a broad
-profile gpu
you can apply that will work everywhere, I think nearly every GPU enabled system has some nuance to it about containerOptions and flags. It would be like setting the
queue
directive to
gpu
, it's a big assumption to make
g
Hi @Jonathan Manning, I'm back today and catching up on this thread and reading up on the pipeline config documentation to better understand the problem and the solution.
j
Comment from Isaac (who reported the original bug on rnaseq):
Copy code
Thanks for the fix!

Just a matter of preference, and again not that familiar with nextflow yet so I am probably missing context, but I think requiring a flag --gpu for only some configurations confusing. I would think that saying "use the GPU aligner" or "this step uses a GPU" would be enough signal from the user.

I am also a little confused on how this will effect specifying the number of GPUs I want a task to use. I had thought that passing an accelerator requirement would have communicated to slurm to pass a GPU resource requirement, but I'm not sure that is happening. I'm going to play around with this next week and see how this works, since the parabricks team had recommended going with two GPUs per alignment task.
g
I like this implementation that uses
containerOptions
to set the gpu container flags, but I'm not sure I fully understand the purpose of the
--use-gpu
flag or what extra functionality that it adds.
j
containerOptions
is always set on
process_gpu
tasks, but the flags inside (
--gpus all
,
--nv
) only activate when
--use_gpu
is true. Without the gate, every Parabricks run would inject GPU container flags, which breaks on cloud executors (AWS Batch, Google Batch, K8s) that handle GPU passthrough via their own APIs.
--use_gpu
lets local/HPC users opt in to container-level GPU flags while cloud users just set
--use_parabricks_star
and let the executor handle it.
@Famke Bäuerle :
setting the accelerator lead to to runs breaking on my cluster which is why we only set it for specific executors.
Did you ever report this as a Nextflow bug?
👎 1
a
I would think that saying "use the GPU aligner" or "this step uses a GPU" would be enough signal from the user.
I agree with this. If someone specifies they want to use Parabricks, that should be enough to enable any GPU features. Having a specific GPU flag only makes sense if you have byte-level CPU duplication and want to accelerate it with GPU hardware. I don't think this exists yet? Sarek does this reasonably well with
--tools
and
--skip-tools
, after all, if you specify
--tools parabricks
you must use GPUs.
party this 1
👍 1
m
one could be using deepvariant within parabricks or not
so gpus or not could be possible, but I am unsure if we have deepvariant parabricks in sarek to be honest
a
Ah well...
--tools parabricks_deepvariant
?
party this 1
I guess this is up to the pipeline author in some ways
m
yeah, that's what I was about to mention, that we could always resolve it in a smart way
I guess this is up to the pipeline author in some ways
Yeah, and that's what is troubling me
I think we as maintainers of the flagship pipelines should converge on the GPU usage and lead the rest of the community to follow
👀 1
👍 1
👍🏾 1
a
Why doesn't Sarek, as the biggest pipeline, simply consume the others?
😂 1
sarek wine 1
👌 1
j
Don't poke the beast Adam!
sarek cat slap 1
m
We will get there at some point 😆
I never hid my plans for world domination
sarek hypnotize 1
a
Well let's focus on rnaseq for now. It's only 1 parabricks tool so far?
m
currently in rnaseq we control the tools via
run_whatever
and
skip_whatever
, right? so more or less that's a similar approach than what we do in sarek, so we could follow a similar set up and check if we have gpu if the specified params are on
j
Just tell me what you want what you really really want guys. I don't really have strong opinions 🙂
a
Apparently my opinion is 90% of it should be able to be controlled by
params
m
How is it done in #CP3RJSMF0?
we have #C04SDJYV12L there as well
g
I am digging into the code now, but they say this:
Copy code
Note: For faster CPU runs with BWA-Meth, enable the BWA-MEM2 algorithm using --use_mem2. The GPU pathway (Parabricks) requires -profile gpu and a container runtime (Docker, Singularity, or Podman); Conda/Mamba are not supported for the GPU module.
If they're using
-profile gpu
then maybe they will run into the same CPU issue that Isaac originally flagged
m
I am agreeing with you there
Which is why I was getting worried about migrating this gpu profile, but I tend to agree with @Adam Talbot it might have been overkill and using just params might do the trick
j
Not all that keen on this @Adam Talbot :
Copy code
params.gpus_per_parabricks = 2
accelerator = params.gpus_per_parabricks
ext.args = "--num-gpus ${gpus_per_parabricks}"
Feels like we need something closer to
task.cpus
👍 1
nod 1
(which is why I liked the
ext
if folks would let me have it)
m
That makes a good case for it
j
How can I get 'official' approval for that new
ext
?
m
Let me ask the wise people that leads #C043UU89KKQ
We'll get back to you
a
Better to stick to this for parabricks then:
Copy code
def num_gpus = task.accelerator.request ? "--num-gpus ${task.accelerator.request}" : '' // or --num-gpus 1 as default?
I think introducing a new variable
task.ext.num_gpus
which duplicates the
task.accelerator.request
value doesn't achieve much?
party this 1
j
As long as we're satisfied that this will not in fact break things where folks were saying it did
a
Use one of those safe ternary things?
Copy code
def num_gpus = task.accelerator?.request ? "--num-gpus ${task.accelerator.request}" : '--num-gpus 1'
💯 1
j
No, I mean this
setting the accelerator lead to runs breaking on my cluster which is why we only set it for specific executors
a
It's possible someone doesn't set process.accelerator, since it's ignored by around of the executors, but we can just set it anyway, as the value for GPUs.
That's gotta be the exception rather than the rule? Anyway, in this case you can just override it for your cluster:
Copy code
process {
    withName: PARABRICKS_STAR {
        accelerator = null
        ext.args = [
            task.accelerator?.request ? "--num-gpus ${task.accelerator.request}" : '--num-gpus 1',
            // stuff
        ].join().strip()
    }
}
m
@Famke Bäuerle does this seems acceptable for you?
f
I can adapt the cluster config to overwrite accelerator. So I do not really care. But I am honestly a bit confused by now which directives we need and which we don't because all the infrastructures also handle GPUs differently (some clusters have a GPU queue some do not, some clouds need it enabled before some do not etc.)
m
should we do a bytesize about this?
j
To summarise: There are two separate concerns: 1. Container GPU flags (
--gpus all
for Docker,
--nv
for Singularity) - these tell the container runtime to expose GPUs to the process. Only needed on local/HPC. Cloud executors (AWS Batch, K8s etc.) handle this via their own APIs. 2.
accelerator
directive - tells Nextflow how many GPUs a task needs. On cloud executors this drives provisioning. On SLURM/local it's silently ignored (confirmed from Nextflow source), but the value is still available in the script via
task.accelerator.request
so tools like pbrun can read it for
--num-gpus
. The plan: -
containerOptions
on
process_gpu
handles #1, auto-detecting the right flags per container engine, gated so cloud executors don't get them -
accelerator = 1
set unconditionally on
process_gpu
handles #2 - Drop
--use_gpu
as a separate param in rnaseq, since
--use_parabricks_star
already signals GPU intent (we might need to tweak later if we get extra GPU things) - No new
ext
attributes - pbrun reads
task.accelerator.request
for
--num-gpus
(matching existing modules, I closed the modules PR) - Users who want multiple GPUs per task (e.g. Parabricks team recommends 2) override
accelerator = 2
in their config -
task.accelerator.request
feeds
--num-gpus 2
to pbrun automatically, and
--gpus all
already exposes all GPUs to the container - If a cluster needs to override any of this (e.g.
accelerator = null
), a
withName
block in the local config handles it
j
I think this should be discussed in a maintainers or ad-hoc meeting were all the people involved in gpu-powered pipeline could discuss the pros and cons of each approach and we should come with some sort of standardisation
j
I'm happy to revert to a different approach later, just need something to get an overdue rnaseq release cut for the moment 🙂
💯 1
a
Only needed on local/HPC. Cloud executors (AWS Batch, K8s etc.) handle this via their own APIs.
this isn't true
j
Arg, OK, I'm confused
a
Don't worry about specifics of each executor, there are too many options
you'll go mad
j
a
YES yes
👍 1
m
yes YES
let's keep it simply simple
💯 1
a
Copy code
process {
    withLabel: 'GPU' {
        accelerator = 1
        containerOptions = { params.gpu_container_options ?: (workflow.containerEngine in ['singularity', 'apptainer'] ? '--nv' : '--gpus all') }                 
    }
    withName: 'PARABRICKS_.*' {
        ext.args = { 
            [
                task.accelerator?.request ? "--num-gpus ${task.accelerator.request}" : '--num-gpus 1',
                // Parabricks args
            ].join().strip()
        }
    }
}
meow love 1
j
I'm going to start losing my will to live soon guys, stop thinking of new things!
😆 1
paranoid 1
a
I think that ☝️ is what you have right now. Just remove the stuff about "not needing this on Cloud", it's not true.
m
Sorry Jon
#C0A2RDVD9L7 is strong today
a
Checking with the robot, but my understanding is: • AWS Batch ignores containerOptions entirely (nothing to do with GPUs) • K8s uses containerOptions and also uses
acclerator
as a pod selector value and has a device plugin which may handle it • GCP and Azure use container options in
docker exec
, GCP also uses accelerator as a Node requirements selector (e.g. it enables GPU machines) • A couple of clusters turn
accelerator
into a job submission field, but not all. They don't do anything re: containerOptions so you have to supply this.
j
I do not have this in pipeline config:
Copy code
withName: 'PARABRICKS_.*' {
        ext.args = { 
            [
                task.accelerator?.request ? "--num-gpus ${task.accelerator.request}" : '--num-gpus 1',
                // Parabricks args
            ].join().strip()
        }
    }
It's covered in the modules:
Copy code
def num_gpus = task.accelerator ? "--num-gpus ${task.accelerator.request}" : ''
👍 1
a
yup but num_gpus goes into the
args
right?
a
is it a mandatory arg?
a
Looks like an
arg
to me
1.2 Non-file mandatory command arguments
Non-file mandatory arguments or arguments needed to modify the command to make the module run with no error, SHOULD be provided as value channels (for example
lib_type
in salmon/quant) - see ‘Input/output options’ below.
1.3 Optional command arguments
All non-mandatory command-line tool non-file arguments MUST be provided as a string via the
$task.ext.args
variable.
still, if it's mandatory it can be calculated on the fly 🤷 not a huge impact either way.
j
Sorry, I'm confused what you mean. I wrote chunks of that guidance, but this is a resource thing we're handling more like task.cpus. Nobody would put
--threads ${task.cpus}
in ext.args.
So, we're done now, right? RIGHT? https://github.com/nf-core/rnaseq/pull/1711
👀 1
a
🎉 2
😅 2
g
I really like how this turned out. Thank you so much @Jonathan Manning for wrangling us to push a fix quickly!
party this 1
blob np 1
j
Turned out to be les complicated than I thought 😉