Update NCI Gadi config and documentation

[kisarur]

---

name: Update NCI Gadi config and documentation
about: Update nci_gadi.config to support strict syntax, include other improvements, and refine the documentation.

Please follow these steps before submitting your PR:

• If your PR is a work in progress, include [WIP] in its title
• Your PR targets the master branch
• You've included links to relevant issues, if any

Steps for adding a new config profile:

• Add your custom config file to the conf/ directory
• Add your documentation file to the docs/ directory
• Add your custom profile to the nfcore_custom.config file in the top-level directory
• Add your profile name to the profile: scope in .github/workflows/main.yml
• OPTIONAL: Add your custom profile path and GitHub user name to .github/CODEOWNERS (**/<custom-profile>** @<github-username>)

[pontus]

Process doesn't have any of these directives?

[maxulysse]

You can check the documentation here:
https://docs.seqera.io/nextflow/config#process-configuration
https://docs.seqera.io/nextflow/reference/process

[kisarur]

The version of Nextflow installed on Gadi has been modified to introduce these two new directives (see the docs). A note has been added to nci_gadi.md about this too.

[pontus]

That's certainly... creative.

I'd strongly recommend using clusterOptions instead, both because it allows users to bring their own versions of nextflow and use them with the profile (e.g. if there's a lag in providing updated versions required for running some pipeline). It would also help if linting starts complaining for unknown directives (I don't know anything that currently does this, but it seems useful for catching typos).

[kisarur]

Thanks @pontus.

The version of Nextflow installed on Gadi includes more changes than just the introduction of these new directives (e.g. adding support for job submission and monitoring with the route/exec queue structure available on Gadi). Therefore, users may not be able to bring their own unmodified version of Nextflow and expect it to work on Gadi.

Also, with Gadi's PBS Pro, storage locations must be specified using -l storage resource directive in the PBS script. If we just add -l storage=${params.storage} to clusterOptions, Nextflow will override the resources specified via directives like cpus, memory, etc. A workaround would be to reconstruct the entire -l resource directive within clusterOptions (e.g., -l storage=${params.storage},ncpus=${task.cpus},mem=${task.memory},...), but I feel our current approach is cleaner.

[jfy133]

@kisarur I sort of feel uncomfortable with it (was it not possible to make a plugin rather than patch nextflow?) but it thats what you need then we have to accept that.

However please could you comment very clearly everywhere in the config wherever not standard nextflow is being used.

We would not want other users copying it over into their configs where they are not using your special patched version of nextflow.

[pontus]

Thanks for the info, I also would feel rather uncomfortable with that setup but as there's no linter complaint I guess it's fine for now. I see Maxime has suggested comments to mark this as special (hopefully meaning people won't try to copy it), it would be great if you implement those.

[awgymer]

I would agree with James that this seems much better suited to a plugin that provides a custom executor than simply monkey-patching nextflow.

It would be possible that custom process directives would still exist then but I think it would be far clearer for users and the plugin + executor would be embedded in the config too which gives additional indication that things are non-standard

[mattdton]

If it helps, we have been supporting nextflow since before plugins were available. This is how it has been done for a while and the initial focus was around getting nextflow working at all.

[maxulysse]

Could you update conf/pipeline/proteinfold/nci_gadi.config as well please?

[kisarur]

@maxulysse The current authors are already working on updating conf/pipeline/proteinfold/nci_gadi.config

[jfy133]

One last comment to really make sure it's very clear this is a non-standard version of Nextflow, otherwise LGTM. Thanks for your patience @kisarur !

[pontus]

@nf-core-bot fix linting

[pontus]

Is this ready for merge now?