Unify iGenomes index usage

[drpatelh]

It appears that we are staging the index files from the standard igenomes.config in different ways in different pipelines. It would be nice to unify this and to use the same logic in the pipeline code. It can often be a source of confusion and I think its about time we come up with a robust solution.

One possible solution is to add the index prefix in igenomes.config for all types of indices (this is only done for BWA at the moment):

tools/nf_core/pipeline-template/{{cookiecutter.name_noslash}}/conf/igenomes.config

Line 15 in d435406

bwa = "${params.igenomes_base}/Homo_sapiens/Ensembl/GRCh37/Sequence/BWAIndex/genome.fa"

and then to split the path into a directory and prefix in the pipeline code so the directory can be staged. This is flexible to custom user-provided paths, instances where the index may be named differently to the genome fasta and works on AWS where globs may not resolve all of the other files in the index (e.g. genome.fa*; @maxulysse ?).
https://github.com/nf-core/chipseq/blob/21be3149542cdc84431e12d1e092359058aed32a/main.nf#L168-L183

It would be nice if we can update and use a single igenomes.config across all pipelines if possible i.e. updating the template version and having this rolled out to pipelines via the automated sync. May be worth adding in Bowtie 1 index paths used in smrnaseq (Ping @lpantano). Also, ping @maxibor who is using Bowtie2 in coproid.

[maxulysse]

Hi,

As we all know glob does not work on URLs, but it does work on s3.
So this works pretty well for the pipeline in production (verified with Sarek):

https://github.com/nf-core/sarek/blob/98ab4996971d248f30bad3102a1bd663056347df/conf/igenomes.config#L35

But obviously not for CI, so indexes will be re-built, but at least you can avoid this hacky part:

https://github.com/nf-core/chipseq/blob/21be3149542cdc84431e12d1e092359058aed32a/main.nf#L176-L183

[drpatelh]

So this works pretty well for the pipeline in production (verified with Sarek):

https://github.com/nf-core/sarek/blob/98ab4996971d248f30bad3102a1bd663056347df/conf/igenomes.config#L35

How would you provide the index on the command-line? and does your implementation account for instances where the fasta file and index files have different prefixes?

e.g. it looks like $fasta and $bwaIndex files have to have the same prefix?
https://github.com/nf-core/sarek/blob/98ab4996971d248f30bad3102a1bd663056347df/main.nf#L1037

But obviously not for CI, so indexes will be re-built, but at least you can avoid this hacky part:

How dare you @maxulysse 😱 I thought that implementation was quite a good and elegant alternative.

[maxulysse]

How would you provide the index on the command-line?

With:

--bwa 'path/to/fasta.{alt,amb,ann,bwt,pac,sa}'

and does your implementation account for instances where the fasta file and index files have different prefixes?

Actually in that case, it just happen to be the same prefix, but it won't matter if it's the same or not as Sarek is not checking for any prefix.
But I believed that bwa indexes had their name from the fasta file, hence the same prefix in the end.

How dare you @maxulysse 😱 I thought that implementation was quite a good and elegant alternative.

I would say it's a crafty hack.
But as a fellow developer once told me: Every cockroach is beautiful to its mother ;-)

[jfy133]

In a tangential topic in eager, something similar came up.

I proposed a slightly alternative to the above. Given BWA index suffixes are always fixed for both aln and mem, a user shouldn't have to manually define them.

Instead I proposed something like this (dodgy pseudocode). Therefore the suffixes are added in the backend.

## User
--fasta '/path/to/mammoth.fasta'
--bwa_index_exists

// main.nf
bwa_index_exists? params.bwa_index = file("${params.fasta}.{amb,ann,bwt,pac,sa}") : NA

[maxulysse]

I'm happy with whatever as long as we remove the crafty hack

[ewels]

Potentially relevant: #592 - RefGenie integration will hopefully mean that people write --bwa <path> when running pipelines less and less (though we'll still need this option).

[ewels]

Conclusion: @maxulysse wins! 🏆

Let's go for explicit globs, update the template iGenomes.config ASAP, preferably before the next release (force all pipelines to update their code).

[maxulysse]

I'll help out with updating the template

[ewels]

@jfy133 #522 (comment) - I think this is basically what @drpatelh / me / most of the nf-core pipelines currently do.

eg. for nf-core/methylseq (code here)

Channel
    .fromPath("${params.bwa_meth_index}*", checkIfExists: true)
    .ifEmpty { exit 1, "bwa-meth index file(s) not found: ${params.bwa_meth_index}" }

[jfy133]

Yeah @apeltzer did something similar, however we were having problems with AWS and path() etc, and I wondered if it was because we giving a directory rather than an actual file that was the issue.

I was accidently roped into this discussion ;).