Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggest adding the process "when:" as a process directive. #2518

Closed
mahesh-panchal opened this issue Dec 22, 2021 · 11 comments
Closed

Suggest adding the process "when:" as a process directive. #2518

mahesh-panchal opened this issue Dec 22, 2021 · 11 comments

Comments

@mahesh-panchal
Copy link
Contributor

mahesh-panchal commented Dec 22, 2021

New feature

The proposal is to add when as a process directive as an alternative to the process when: block.

Usage scenario

It allows configuration of when a process executes directly from the configuration, allowing a set of processes to be labeled and then executed as a set, for example running a set of processes that use GPU's otherwise use processes that use CPU's.
Alternatively, running a process if a param's is set without putting it directly in the process definition.

Suggest implementation

Using the same functionality of a when: but that conditional would be in a closure, while otherwise it would be resolved directly in the config.

@mahesh-panchal mahesh-panchal changed the title Suggest adding the process "when: as a process directive. Suggest adding the process "when:" as a process directive. Dec 22, 2021
@bentsherman
Copy link
Member

If you are using DSL2, you can also just use if statements to control when processes get invoked. DSL2 has kinda made the when: block obsolete.

@mahesh-panchal
Copy link
Contributor Author

There are cases when it can still be useful though. While I agree if or channel branching can solve many cases, there are some where it cannot help such as testing configuration settings. A real world example is from the nf-core viralrecon workflow which in a previous version used to pass software specific parameters via a params.modules map, testing for a particular setting ( a flag if (params.spades_options.args.contains('--meta') ||). However in the most recent version of nf-core workflows, software specific parameters are now passed using the process directive process.ext.args which has it's benefits, but cannot be accessed in the workflow script block. This should still be accessible in the when: block though.

@bentsherman
Copy link
Member

I see. Since the ext directive adds properties to the task object, the when: block just needs to be able to access the task object:

when: task.ext.args.contains('--meta')

That would actually be a real use case for using when: over if statements in the workflow block. Have you tried this? It may be possible already.

@mahesh-panchal
Copy link
Contributor Author

It works using a when:.
main.nf:

#! /usr/bin/env nextflow

nextflow.enable.dsl = 2

workflow {

    FOO()

}

process FOO {

    echo true

    when:
    !task.ext.args.tokenize().contains('help')

    script:
    def args = task.ext.args?: ''
    """
    echo $args
    """
}

nextflow.config:

process {
    withName: 'FOO' {
        ext.args = 'save me'
        // ext.args = 'help me'
    }
}

However, to put this back into context, nf-core modules don't include when: statements to make them generic and usable between pipelines. Therefore it would be nice if this was a configuration option, i.e. specifically a process directive where one could add conditionals such as these based on task data.

@bentsherman
Copy link
Member

So how does the viralrecon workflow handle this issue if it can't use when: ? Or does it just not use this logic anymore?

Perhaps the nf-core guidelines should be changed to allow the use of when: only when the condition is based on the task object.

@mahesh-panchal
Copy link
Contributor Author

It looks like another workflow parameter was added to viralrecon to take care of it, and the logic is based on that.

One of the aims of using nf-core modules is that there shouldn't need to be any modifications to the modules. They should be just plug and play. Any modifications mean maintaining a local copy in the workflow instead. I think someone is developing a patch system for modules, but I can't remember it's original intent.
@drpatelh is the one generally driving how nf-core modules should be structured, and the development of viralrecon

@drpatelh
Copy link
Contributor

drpatelh commented Feb 2, 2022

Yes, it is actually a guideline not to use the when: statement in any module mainly because if we want to allow re-use then it makes sense to avoid using any custom logic - keeps the modules nice and clean.

The only other alternative is to use if/else statements which is what we are now doing in viralrecon and other DSL2 pipelines.

@bentsherman
Copy link
Member

@drpatelh What do you think about this situation with process.ext.args ? I don't see how you check such values without using when:. It also seems like using when: in this case wouldn't affect the re-usability of a module.

@drpatelh
Copy link
Contributor

drpatelh commented Feb 2, 2022

It also seems like using when: in this case wouldn't affect the re-usability of a module.

Yup, you're right. In this case, it wouldn't matter much but it still begs the question of whether one man's when is another man's when? Would we also have to evaluate whether task.ext.args is set (null) somehow - needs to be tested 🤔

I suspect we will see more of this now though where we will need to somehow evaluate the task.* attributes outside of the modules scope. We can set task.* attributes really easily already.

@mahesh-panchal
Copy link
Contributor Author

Would we also have to evaluate whether task.ext.args is set (null) somehow

Yes. The safe navigation operator isn't suitable here, but just using && will work.

when:
task.ext.args && !task.ext.args.tokenize().contains('help')

@mahesh-panchal
Copy link
Contributor Author

And I just realized now we can implement this ourselves....

    when:
    task.ext.when == null || task.ext.when

see: nf-core/tools#1393

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

No branches or pull requests

3 participants