Patch to allow longer runtimes on large genomes

[tbrown91]

PR checklist

• This comment contains a description of changes (with reason).
• If you've fixed a bug or added code that should be tested, add tests!
• If you've added a new tool - have you followed the pipeline conventions in the contribution docs
• Make sure your code lints (nf-core pipelines lint).
• Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
• Check for unexpected warnings in debug mode (nextflow run . -profile debug,test,docker --outdir <OUTDIR>).
• Usage Documentation in docs/usage.md is updated.
• Output Documentation in docs/output.md is updated.
• CHANGELOG.md is updated.
• README.md is updated (including new tool citations and authors/contributors).

[sainsachiko]

LGTM!

[DLBPointon]

I don't think we actually need this unless it's a very common thing.

you can write a something.config file containing:

process {
    time = { 2.h * task.attempt }
}

pass that to the pipeline with -c something.config

That way you can get it running now.

[sainsachiko]

I don't think we actually need this unless it's a very common thing.

you can write a something.config file containing:

process {
    time = { 2.h * task.attempt }
}

pass that to the pipeline with -c something.config

That way you can get it running now.

Yes, this could be a nice workaround if we want to finish those runs without a release.
Actually 1h or 2h seems make no different regarding the job queue to me (Both go into normal queue as I remember ^^ ).
In the long term for the pipeline, like Damon mentioned, if this is common case, we can either increase this or set a condition for timing based on genome size like here.

[tbrown91]

Longer runtime worked, but I'm now getting out of memory issues in the tabix step. The rule increases with number of sequences, but not total genome or largest scaffold size, which is important for those that require the --csi indexing

sequencecomposition/conf/base.config

Line 22 in 0cd2b0a

memory = { 50.MB * task.attempt * Math.ceil(meta.n_sequences / 25000) }

[sainsachiko]

Longer runtime worked, but I'm now getting out of memory issues in the tabix step. The rule increases with number of sequences, but not total genome or largest scaffold size, which is important for those that require the --csi indexing

sequencecomposition/conf/base.config

Line 22 in 0cd2b0a

memory = { 50.MB * task.attempt * Math.ceil(meta.n_sequences / 25000) }

Thanks for noticing this, @tbrown91. For now can you use extra config to increase memory for the TABIX? The pipeline currently compute n_sequences, total_length, max_length, so memory can be adjusted based on these when we know memory usage of TABIX across pipeline runs.

[DLBPointon]

So a tbi file is generated, under x. A csi and tbi generated under y and just a gzi above 2 ** 32?

Can we not just use CSI? The trade off is pretty small right, the pipelines we've done in ToLA default to just csi as its the flexible format. Do you know at all @sainsachiko ?

process {
    time = { 2.h * task.attempt }

    withName: BGZIPTABIX {
        memory = { 50.MB * task.attempt * (max_seq_length >  2**32 ? Math.ceil(meta.n_sequences / 5000) : Math.ceil(meta.n_sequences / 25000)) }
    }
}

This way, if size is big enough to need csi then it gets 5* the memory. Potentially overkill but should get the job done

[sainsachiko]

So a tbi file is generated, under x. A csi and tbi generated under y and just a gzi above 2 ** 32?

Can we not just use CSI? The trade off is pretty small right, the pipelines we've done in ToLA default to just csi as its the flexible format. Do you know at all @sainsachiko ?

process {
    time = { 2.h * task.attempt }

    withName: BGZIPTABIX {
        memory = { 50.MB * task.attempt * (max_seq_length >  2**32 ? Math.ceil(meta.n_sequences / 5000) : Math.ceil(meta.n_sequences / 25000)) }
    }
}

This way, if size is big enough to need csi then it gets 5* the memory. Potentially overkill but should get the job done

Thanks Damon, it’s really great to have a ratio that we know works in practice! I really like your suggestion, perhaps current OOM runs could be proceeded using this configuration.

I was thinking we could then use a few different datasets to get a better sense of a reasonable scaling ratio to put on dev/main. It would be nice to have something that works well across a range of datasets rather than over-provisioning from the start. Also, since this pipeline is mainly maintained by @muffato, it would be great to get his thoughts too.

Regarding CSI, as in a GAP context, the pipeline is currently output all 3 types of indices (TBI, CSI, GZI) for all outputs where possible (example outputs).

[muffato]

This 50.MB + 30.min is low on purpose. It's there to force pipeline steps to define their own resources if they need more than a trivial amount. It's a practice I've introduced when walking on pipeline resource optimisation.

If you see that a step sometimes needs more than 30 min, it's absolutely fine to write it down in the pipeline config, but in a process-specific block rather than the catch-all block.
ChatGPT could give some insights in what the peak memory is: Tabix Memory Complexity.pdf

Also, it's often difficult to pinpoint the exact memory usage. That's why we tend to insert a * task.attempt (or similar) somewhere so that when the retry mechanism kicks in, the job is given more resources and hopefully completes.

Regarding TBI and CSI, I'm generating both in all pipelines because I'm not just which one is used. I fear some tools only support TBI ... I don't have enough experience of bioinformatics in the wild to know ...

[tbrown91]

Based on the document you linked, the length of the longest sequence should also be included in the calculation, something like * max_seq_length/2**16

Regarding the indices: I think this makes sense to include all of them