Skip to content

feat: add dorado/aligner module - #11222

Open
sahuno wants to merge 5 commits into
nf-core:masterfrom
sahuno:add-dorado-aligner
Open

feat: add dorado/aligner module#11222
sahuno wants to merge 5 commits into
nf-core:masterfrom
sahuno:add-dorado-aligner

Conversation

@sahuno

@sahuno sahuno commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

Description

Adds dorado/aligner — a wrapper around Oxford Nanopore's dorado aligner, which uses minimap2 under the hood to align unaligned ONT BAMs (e.g. produced by dorado/basecaller) while preserving modified base tags (MM/ML) and other BAM auxiliary tags.

CPU-only (no GPU required) — dorado aligner wraps minimap2 which is CPU-native. GPU is only needed for basecalling.

Why a separate module from dorado/basecaller?

Follows nf-core convention of one tool-subcommand per module. This is also the approach recommended by @Kevin-Brockers / @dialvarezs in #11122 (dorado/basecaller review). Keeping basecall and align separate lets users:

  • Use different CPU/memory profiles (basecall needs GPU, aligner doesn't)
  • Swap the aligner for minimap2/align if preferred
  • Resume the pipeline from the aligner step without re-basecalling

Test data

Test paths depend on nf-core/test-datasets#1969 (unaligned HG002 GIAB 10-read BAM). Tests were verified locally against the same files before PR; CI will go green once #1969 merges.

Verified real-test output:

[info] Running: "aligner" "--threads" "2" "genome.fasta" "HG002_PAW70337_giab_10reads.unaligned.bam"
[info] > Reads written: 10
[info] > total/primary/unmapped 21/6/4
[info] > Finished in (ms): 107

Snapshot strategy

Real-test snapshot intentionally captures only filename + dorado version (not BAM MD5), because dorado aligner embeds absolute paths in @SQ UR: and @PG CL: BAM header lines — these vary between test environments. Stub-test snapshot uses full process.out (stable because all files are empty touch'd).

PR checklist

  • Description of changes (above)
  • New tool — followed module conventions (main.nf, meta.yml, environment.yml, tests/)
  • Test data — uses files from Add ONT BAM and bedMethyl test data: HG002 GIAB 10-read subset (PAW70337) test-datasets#1969
  • No TODO statements
  • Versions broadcast via topic: versions
  • Naming conventions followed
  • Input/output options per guidelines
  • Resource label: process_high (CPU/RAM scales with input)
  • Container: docker.io/nanoporetech/dorado:shac8f356489fa8b44b31beba841b84d2879de2088e (ONTPL license — not on bioconda; vendor container pattern matches nf-core/parabricks)
  • nf-core modules test dorado/aligner --profile singularity
  • nf-core modules lint dorado/aligner ✅ (2 expected warnings re: vendor Docker Hub image, same as dorado/basecaller)

Conda and Docker profile tests not yet run (dorado not on bioconda; Docker not available on my dev host — expecting CI to validate).

🤖 Generated with Claude Code

Wraps `dorado aligner` (minimap2) to align unaligned ONT BAMs (e.g. from
dorado/basecaller) while preserving modification tags (MM/ML). CPU-only —
no GPU required.

- main.nf: DORADO_ALIGNER process, process_high label, vendor container
- meta.yml: EDAM ontologies for BAM/FASTA/FAI/TSV inputs and outputs
- tests: stub + real test against GIAB HG002 unaligned BAM and hg38 slice
  (snapshot reduced to stable fields to avoid BAM header path drift)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sahuno and others added 3 commits September 6, 2026 12:58
…skip conda

All six nf-tests were failing because the fixture they referenced,
`HG002_PAW70337_giab_10reads.unaligned.bam`, was never merged into
nf-core/test-datasets (it is still pending in nf-core/test-datasets#1969), so
`checkIfExists: true` threw before the process ever ran.

Tests now use `test2.sorted.bam`, which is already merged and maps to the same
chr22 subset reference. `test.sorted.phased.bam` cannot be used: each of its
records carries two RG tags (one from minimap2 `-R`, one from the original
dorado basecall) while its header declares only one. dorado preserves both, so
the output BAM trips htsjdk's READ_GROUP_NOT_FOUND check inside `getReadsMD5()`.

Other fixes found while verifying against dorado 1.4.0:

- The `summary` output could never be produced. `--emit-summary` writes a
  fixed-name `sequencing_summary.txt`, not `*_summary.tsv`. The glob is
  corrected and the script renames the file to `<prefix>.sequencing_summary.txt`
  so outputs stay per-sample. The test now sets `--emit-summary` and asserts the
  channel is populated.
- Removed the dead `versions.yml` heredoc from the stub: this module reports
  versions through the `versions` topic, so nothing consumed that file.
- Removed a no-op `sed 's/^//'` from the version eval.
- The real test now actually computes the reads-only MD5 its comment promised,
  via nft-bam's `getReadsMD5()`, and snapshots it under a `versions` key so the
  `test_snap_versions` lint check passes.
- Corrected the `.fai` input ontology, which pointed at the EDAM term for BED.

Added `modules/nf-core/dorado` to the conda skip list, matching how parabricks
is handled: dorado is not on bioconda (ONTPL licence) so the conda profile can
never resolve it. docker and singularity are unaffected.

Remaining known failure is `container_links`: nf-core lint only accepts
`quay.io` or `community.wave.seqera.io/library` prefixes, and dorado is
published only on Docker Hub. Flagged for maintainer input.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E7RSvpouxbMBdimYSwJ5ZP
…tection

Two remaining lint failures, both now fixed:

`container_links` failed because `nf-core lint` only accepts prefixes listed
under `container-registry:` in `.nf-core.yml`. Added `docker.io/nanoporetech`
next to the existing `nvcr.io/nvidia/clara` entry that lets the parabricks
modules use NVIDIA's registry.

`docker_tag` and `singularity_tag` failed with "No container was extracted".
The cause is obscure: lint resolves the container by running `nextflow inspect`,
and Nextflow was rejecting this file with "DSL1 is no longer supported". The
trigger was the word "from" in the `//` comment on the input BAM line — inside
an `input:` block it makes Nextflow's DSL-version heuristic read the file as
DSL1. Rewording the comment to "produced by" resolves it. Verified by bisection:
changing only that word flips `nextflow inspect` between failing and returning
the container.

Also switched the snapshot from nft-bam's `getReadsMD5()` to the record count.
dorado writes records in a thread-dependent order with `SO:unknown`, so the
reads md5 differs between runs on identical input (8dba95c6… vs c436519161…).
The count is stable; verified over three consecutive runs.

nf-core/tools 4.1.0 lint: 56 passed, 0 warnings, 0 failed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E7RSvpouxbMBdimYSwJ5ZP
@sahuno sahuno mentioned this pull request Sep 6, 2026
11 tasks
@sahuno

sahuno commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

This PR had all six nf-tests plus lint failing. All of it is now fixed and it is
ready for review.

Why every test failed: the fixture it referenced,
HG002_PAW70337_giab_10reads.unaligned.bam, was never merged into
nf-core/test-datasets — it is still pending in nf-core/test-datasets#1969. So
checkIfExists: true threw before the process ever ran.

Rather than block on that, the tests now use test2.sorted.bam, which is
already merged and maps to the same chr22 subset reference. Worth recording why
it is not test.sorted.phased.bam: every record in that file carries two RG
tags (one from minimap2 -R, one from the original dorado basecall) while its
header declares only one. dorado faithfully preserves both, so the output BAM
trips htsjdk's READ_GROUP_NOT_FOUND check. test2.sorted.bam has no RG at all
and is clean.

Why lint failed, in two parts:

  1. container_linksnf-core lint only accepts prefixes listed under
    container-registry: in .nf-core.yml. Added docker.io/nanoporetech
    beside the existing nvcr.io/nvidia/clara entry that lets parabricks use
    NVIDIA's registry. (Add module: dorado/basecaller #11122 adds the same line; whichever merges second may
    need a trivial conflict resolution.)
  2. docker_tag / singularity_tag failed with "No container was extracted".
    This one is obscure and worth flagging for others: lint resolves the
    container by running nextflow inspect, and Nextflow was rejecting the file
    with "DSL1 is no longer supported". The trigger was the word "from" in
    the // comment on the input BAM line — inside an input: block it makes
    Nextflow's DSL-version heuristic read the file as DSL1. Rewording the comment
    fixes it. Verified by bisection: changing only that one word flips
    nextflow inspect between failing and returning the container.

Other fixes found while verifying against dorado 1.4.0:

  • The summary output could never fire: --emit-summary writes a fixed-name
    sequencing_summary.txt, not *_summary.tsv. Corrected the glob; the script
    renames it to <prefix>.sequencing_summary.txt. The test now sets
    --emit-summary and asserts the channel is populated.
  • Removed the dead versions.yml heredoc from the stub (this module uses the
    versions topic) and a no-op sed 's/^//' from the version eval.
  • Corrected the .fai input ontology, which pointed at the EDAM term for BED.
  • Added modules/nf-core/dorado to the conda skip list, the same way parabricks
    is handled — dorado is not on bioconda (ONTPL licence), so the conda profile
    can never resolve it. docker and singularity are unaffected.
  • The real test now snapshots the record count rather than nft-bam's
    getReadsMD5(): dorado writes records in a thread-dependent order with
    SO:unknown, so the reads md5 differs between runs on identical input
    (8dba95c6… vs c436519161…). The count is stable — verified over three
    consecutive runs.

nf-core/tools 4.1.0 lint: 56 passed, 0 warnings, 0 failed. Both nf-tests
pass under the singularity profile.

sahuno added a commit to sahuno/modules that referenced this pull request Sep 11, 2026
dorado is not distributed on bioconda or conda-forge (ONTPL licence), so
`environment.yml` is a placeholder carrying only samtools. The conda
profile run would therefore execute `dorado basecaller` against an
environment that does not contain dorado.

Adds the `modules/nf-core/dorado` prefix to the `conda` skip list, the
same mechanism parabricks and cellranger use. Matches the entry already
present in nf-core#11222 (dorado/aligner) so each PR stands alone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013fqrTt7TV2RSSUHaA4wP4v
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013fqrTt7TV2RSSUHaA4wP4v
@sahuno

sahuno commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

Posting CI evidence, because I think this needs a decision on the container rather than another re-run.

nf-test fails on the singularity lane before any test logic runs — including the -stub tests, since Nextflow pulls the image to run a stub. Two different failure modes, from two runs of the same commit:

FATAL: While making image from oci registry: error fetching image to cache:
  while building SIF from layers: conveyor failed to get: error writing layer:
  stream error: stream ID 11; INTERNAL_ERROR; received from peer
FATAL: ... unpack entry: opt/custflow/epi2meuser/dorado/lib/libtorch_cpu.so:
  unpack to regular file: short write: no space left on device

On a re-run one shard went green and the other failed with the disk error, so it looks like flake but is really size. Measured from the registry:

  • docker.io/nanoporetech/dorado:shac8f3564… is 9.4 GB over 28 layers, with a single 6.95 GB layer (libtorch).
  • All 45 tags of that image are between 6 and 12 GB — ONT bundles the CUDA runtime and the basecalling models, and publishes no slim variant.

Singularity needs roughly 2–3× the image size on disk to convert OCI layers to a SIF, which is what exhausts the runner.

Worth noting for this module specifically: dorado aligner is minimap2 under the hood and runs on CPU. It needs neither CUDA nor the models, so essentially that whole 6.95 GB layer is dead weight here.

Options as I see them, and I am happy to do the work on whichever you prefer:

  1. A slim image. Build a CPU-only dorado container carrying just the binary and its runtime libs, frozen to community.wave.seqera.io/library/... via Wave. Should be a small fraction of 9.4 GB.
  2. Host it under the nf-core org, which is what @dialvarezs suggested on Add module: dorado/basecaller #11122 for dorado/basecaller. Same underlying question, so it probably wants one answer for both PRs.
  3. Something you already do for large images that I have missed — parabricks is the closest precedent I could find, and it is much smaller.

@SPPearce @famosab @edmundmiller any preference? I do not want to change the container reference on a PR under review without agreement on where the image should live.

@sahuno
sahuno marked this pull request as ready for review September 11, 2026 02:50
@sahuno
sahuno requested review from a team as code owners September 11, 2026 02:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant