Conversation
Swap the short-read mapper alternative behind --bwa from bwa to bwa-mem2, mainly for the faster index build of large indices combining several eukaryotic genomes. The --bwa parameter and all result files/names are unchanged. - rename modules/bwa.nf -> modules/bwamem2.nf (bwa_index/bwa -> bwamem2_index/bwamem2, labels bwamem2_index/bwamem2) - new container nanozoo/bwa-mem2:2.3--4533398 - envs/bwamem2.yaml with bwa-mem2 2.3 and samtools/htslib 1.24 - separate label for the index step: it is single-threaded, so it no longer reserves 24 CPUs, but it gets more memory than the mapping step - a bwa-mem2 index is ~4x larger than a bwa index and is held in memory, so the memory requests in node.config went up - add the mapper to conda.config (it had no environment with the conda/mamba profiles) and to local.config (it had no cpus/memory with the local/standard profiles) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Update all containers and conda environments to the same samtools/htslib version, and pin htslib explicitly so bgzip/tabix are no longer pulled in only as a transitive samtools dependency. - containers: samtools 1.24, minimap2 2.31, bed_samtools 2.31.1, bwa-mem2 2.3 (now with samtools), seqkit 2.13.0 - envs follow the container versions: minimap2 2.26 -> 2.31, bedtools 2.30.0 -> 2.31.1, seqkit 2.6.1 -> 2.13.0, samtools -> 1.24 - envs/minimap2.yaml: pigz 2.3.4 -> 2.8, the old pin cannot be solved next to samtools 1.24 (zlib 1.2 vs libzlib 1.3.2) - envs/seqkit.yaml: tabix 1.11 -> htslib 1.24, which ships tabix and bgzip - samclipy gets an explicit container instead of implicitly taking the one of the smallTask label; the samtools image has the python and git it needs. Its env now pins python 3.14 (as in the container) and adds git, which the process calls but the env never provided. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The CI matrix uses NXF_VER "latest", which now resolves to Nextflow 26.04,
where the strict syntax is the default. That broke all "latest" jobs, on
this branch as well as on main.
Fixed here:
- nextflow.config: `def trace_timestamp` is a variable declaration, which
cannot be mixed with config statements -> moved into the params scope
- clean.nf: typed `for` loops are not part of the syntax -> use `each`
- clean.nf: `addParams()` on include statements was removed. `tool` was
never read anywhere, `lib_pairedness` is now set in the params scope,
which is where the modules read it from anyway
- modules: `if` blocks around publishDir/storeDir are not valid process
directives -> use the `enabled` option of publishDir, and a ternary for
the storeDir of download_host
- modules/utils.nf: `env(TOTALRECORDS)` refers to a shell variable, the
strict parser wants the string form `env('TOTALRECORDS')`
Note that `enabled` needs a real boolean: `enabled: params.keep` silently
publishes nothing, because Nextflow runs a String through
Boolean.parseBoolean().
This is not enough for Nextflow >=26.04 -- the ~180 top-level statements in
clean.nf have to move into the entry workflow first. Until then the CI
matrix is pinned to 25.10.0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The illumina conda job fails reproducibly while creating environments, with either `LockError: Failed to acquire lock` or a package archive that another process removed mid-extraction. Both come from several `conda env create` calls running at the same time against the shared package cache. - executor.queueSize = 1 for the test config, so environments are built one after another. The tasks are stubs, there is nothing to parallelize. - point conda.cacheDir at one shared directory. It defaults to a path relative to the launch dir, and nf-test gives every test its own launch dir, so each of the six illumina tests rebuilt every environment from scratch. Now they are built once and reused. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 1, 2026
check_own always wrote checked.fa.gz, so more than one --own (or --keep) FASTA made concat_contamination fail with an input file name collision. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An --input glob that matches files but forms no read pairs left every process without a task, so the run reported success without cleaning anything. checkIfExists does not catch this. Also give bwamem2_index 8 cpus: the build is not as serial as assumed and uses ~10 cores, so a cgroup-pinned single core would slow it down badly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
minimap2 was handed the reference FASTA directly, so every sample rebuilt the index; for a multi-genome host that dominates the runtime. Add a minimap2_index process mirroring bwamem2_index. -k and -w are baked into the index, so index and mapping have to agree on the preset: minimap2_preset() is now the single source for both, which also collapses the four near-identical mapping commands into one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing in the config declared a time, so every task ran under the default limit of the queue. Mapping a large sample against a multi-genome host exceeded that and SLURM killed the job with exit 140; the retry ladder then only raised the memory, which does not buy more wall time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
index_bam, idxstats_from_bam and flagstats_from_bam ran under the minimap2 label, so each of them asked for 24 cpus, 24 GB and now also a 12 h wall time for a few minutes of work. On a backfill scheduler that delays them behind the mapping jobs they follow. They get the samtools container that the smallTask label already uses and a matching small Conda environment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hopefully also fixing #110