https://nf-co.re logo
Join Slack
Powered by
What are people's thoughts on separating configs i...
# configs
m
What are people's thoughts on separating configs in nf-core/configs into multiple files? We ( @pontus , @Phil Ewels, and I ) have written a config for Uppmax which sets resources based on which cluster is detected. We need to add a new cluster into the mix, and I would also like to update it for the strict syntax. What are the thoughts on splitting the configs into files for each cluster (
pelle.config
,
bianca.config
,
rackham.config
, etc), and one 
uppmax.config
which dynamically includes the config based on cluster name? This would mean users could either use the old way or the cluster specific profile.
m
I'm all for this
I merged some configs together for this profiles, because it was simpler, but no problem for this
p
I think user changes would be bad but have no strong opinions beyond that. Some possible considerations; would care need to be taken to support offline access? Code duplication is obviously bad by I guess we'll need it anyway because of strict syntax.
m
User's shouldn't notice a change other than Pelle would also be supported.
👍 2
p
Is this still running off the 
hostname
trick? I'm always a little nervous about that, but I guess it's ok.. 😅
m
Yes, but I plan to change to the same trick that 
pdc_kth
is using, although I don't have access to Bianca, Snowy, Miarka, or Irma If I can find someone from Uppmax to help me check
either way it's still a similar hack
Actually I should ask in our slack if someone can confirm
🎉 1
p
Basically this?
m
Copy code
sacctmgr show cluster -P -n | cut -f1 -d'|' | grep "bianca\|irma\|miarka\|pelle\|rackham\|snowy"
maybe without the grep
p
(if someone has ideas for anything without relying on external tools, that'd be great, but I can't think of anything good.)
m
The other option is the user supplies the cluster name themselves, but that would change it for the users
p
Only having them as separate configs and the user needing to do 
-profile uppmax_bianca
etc
Yeah, that ☝🏻 (could also do it as a param I guess)
Has anyone tried this with the new strict config parser?
p
I think that would be a needless impairment.
p
Copy code
NXF_SYNTAX_PARSER=v2
The reason I'm nervous is that we're generally moving away from the whole "a config file is an arbitrary groovy script" thing, as it causes a lot of problems
m
I think I did with pdc, but I've been testing too many things lately. I've used the option, and I've tested the pdc config, just not positive if together
👍🏻 1
p
Think 
pyproject.toml
vs. 
setup.py
Ben [2:50 PM]
yeah, what I realized is that because of these self-invoking closures like Mahesh is doing here, you basically have the same level of dynamism in the strict syntax, just isolated in these little bubbles. I see no reason to get rid of that, it's quite a useful feature
shared a GIF
🚀 1
🙌🏽 1
m
Definitely not tested with 
NXF_SYNTAX_PARSER=v2
it seems. I'm hitting issues with template config contents.
Copy code
$ NXF_SYNTAX_PARSER=v2 nextflow run nf-core/rnaseq -profile pdc_kth -r 3.16.1 --project "naiss2024-22-1346" --outdir results

 N E X T F L O W   ~  version 25.04.4

WARN: Cannot read project manifest -- Cause:  Config parsing failed
Error nextflow.config:305:1: Variable declarations cannot be mixed with config statements
│ 305 | def trace_timestamp = new java.util.Date().format( 'yyyy-MM-dd_HH-mm-s
╰     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
I think I linted for the config specific stuff then in that case.
👍🏻 1
p
Can you try 3.19.0 ? I think that should have that one solved..?
m
Interesting. When running without the revision, it told me 3.16.1 was the latest. What's reporting the latest version in that message?
Error's from elsewhere:
Copy code
$ NXF_SYNTAX_PARSER=v2 nextflow run nf-core/rnaseq -profile pdc_kth -r 3.19.0 --project "naiss2024-22-1346" --outdir results

 N E X T F L O W   ~  version 25.04.4

WARN: Cannot read project manifest -- Cause:  Config parsing failed
Error nextflow.config:147:37: `task` is not defined
│ 147 |         path: { "${params.outdir}/${task.process.tokenize(':')[-1].tok
╰     |                                     ^^^^

Error nextflow.config:443:30: `manifest` is not defined
│ 443 | \033[0;35m  nf-core/rnaseq ${manifest.version}\033[0m
╰     |                              ^^^^^^^^

Error nextflow.config:446:26: `manifest` is not defined
│ 446 |         afterText = """${manifest.doi ? "\n* The pipeline\n" : ""}${ma
╰     |                          ^^^^^^^^

Error nextflow.config:446:69: `manifest` is not defined
│ 446 | ? "\n* The pipeline\n" : ""}${manifest.doi.tokenize(",").collect { "  
╰     |                               ^^^^^^^^

Error nextflow.config:446:186: `manifest` is not defined
│ 446 | doi.org/','')}"}.join("\n")}${manifest.doi ? "\n" : ""}
╰     |                               ^^^^^^^^

Error nextflow.config:455:22: `validation` is not defined
│ 455 |         beforeText = validation.help.beforeText
╰     |                      ^^^^^^^^^^

Error nextflow.config:456:21: `validation` is not defined
│ 456 |         afterText = validation.help.afterText
╰     |                     ^^^^^^^^^^


ERROR ~ Config parsing failed

 -- Check '.nextflow.log' file for details
🫠 1
p
Different errors - progress..?
m
In my book a different error is a sign of progress
p
shared a GIF
m
It didn't register earlier with me, but @pontus brings up a good point. How have people been using the Bianca config (offline mode)? Does nf-core download institutional configs too?
p
The errors above with 
task
have already been fixed btw, will be backported soon
🎉 1
The 
manifest
ones are (I think) described here
Does nf-core download institutional configs too?
I believe so, yes. At least, it used to 👍🏻
m
Sorry. I wasn't clear. I mean for offline clusters. If you use 
nf-core download
, does it package the institutional configs too
If I split the config and it has to be fetched separately, then it'll mean more files to download.
And I'm finding there really is a lot of repetition. I might just stick with one config in the end and some ugly code spots
p
My memory says there was some magic for handling that, which would probably need to be adjusted for something loading files separately.
m
The 
nf-core pipelines download
command will download both the pipeline code and the institutional nf-core/configs files. It can also optionally download any singularity image files that are required.
https://nf-co.re/docs/nf-core-tools/pipelines/download
Seems it does as Phil said.
this 1
I started on a draft. https://github.com/nf-core/configs/pull/922/files Any initial comments?
p
Looks quite good to me. I'm somewhat torn on keeping support for old pipelines (and e.g. if 
params.max_cpus
and so on need to be managed).
m
Just to shed some light on the Download part: Yes, one can optionally download the configs - unless the SeqeraPlatform format (bare repositories) is used. It will then also get the whole 
nf-core/configs
repo together with the workflow. The 
nextflow.config
of the pipeline will be edited to change the 
custom_config_base
parameter from 
<https://raw.githubusercontent.com/nf-core/configs/>
to 
../configs
. Since people tend to do not keep the configs in this place, they actually complain about this feature once in while. So from a Download perspective splitting the configs should be fine, as long as these are relative imports to the main profile.
❤️ 1
m
I'm somewhat torn on keeping support for old pipelines
I don't have a good overview of the pipelines and their states. Maybe someone on core team can say better? Do we have a summary anywhere which template/version pipelines are ( not release version ) so we can see which features we can deprecate/remove and the minimum version of Nextflow supported?
The config also has some syntax error that I can't locate. The line looks the same as the dardel one basically except that it's in an 
includeConfig
Ah yeah. nf-core/eager is still dsl1
p
Eager is pretty much the only pipeline that's still DSL1 and is heavily used / maintained, that I'm aware of
(The DSL2 release has been "soon ™️ " for a long time, but really should be pretty soon now)
🤔 1
A matrix of pipelines and their minimum Nextflow version is a good idea 🤔 And for newer pipelines we should also be able to scrape the template version from 
.nf-core.yml
party this 2
m
@Maxime Garcia What's using 
params.single_cpu_mem
? I see this parameter in the sarek uppmax.config. Is it needed?
m
I don't think it's used anymore
p
Made a script for the versions thing.. It's chonking through now..
👍🏽 1
m
let me check
m
It seems we might be able to ditch the non-strict stuff in the sarek uppmax.config.
m
yeah, I think it's pretty old, when sarek was not as close as nf-core standards as it is now
I'm unsure about the location for the references, but I don' think we have the genomes_base params anymore, so it won't work anyway
m
So just delete the config?
m
yeah, we have no genomes_base and no single_cpu_mem anymore
process names are full uppercase, so BamQC shouldn't work
m
@Daniel Lundin Do we still need the ampliseq uppmax config? It looks like it could be updated within the pipeline to be:
Copy code
process {
    withName: classifier_extract_seq {
        cpus           = 1
        time           = 7.d
        // clusterOptions = { "-A ${params.project} -p core -n 1 -t 7-00:00:00 ${params.clusterOptions ?: ''}" }
    }

    withName: classifier_train {
        cpus           = 1
        memory         = 250.GB
        time           = 1.d
        // clusterOptions = { "-A ${params.project} -C fat -p node -N 1 -t 24:00:00 ${params.clusterOptions ?: ''}" }
    }

    withName: classifier {
        memory         = 250.GB
        // clusterOptions = { "-A ${params.project} -C fat -p node -N 1 ${params.clusterOptions ?: ''}" }
    }
}
d
Possible. I'm not using this anymore and afaik there's no corresponding config for pdc_kth so maybe this is not essential. I'm on vacation and replying from my phone so I can't see the consequences of what you propose
m
Enjoy your vacation. It's not important. It can be dealt with another time.
p
@Mahesh Binzer-Panchal - most pipelines are pinning 24.04 as their minimum required version:
💥 1
🙌🏽 1
m
Would it be worth adding a lowest version in a summary box under the chart? Although I can see the minimum we need to support is likely 19.10.0
and Thank you!
p
How do you mean lowest version? That's what the table is showing already, no?
p
If I'm guessing, simply reporting the lowest (required) version seen. Probably not really needed for now, but better than having to go through all entries manually, particularly if they're sorted by number of occurrences rather than by version which could lead to one version being missed if there were more.
💯 1
p
you can sort the table, so it's pretty easy to find
👍🏽 1
sort by version, click a second time to reverse sort, there's your answer..
Open in Slack
PreviousNext
Powered by