Skip to content

feat(semibin): add GPU support to single_easy_bin and multi_easy_bin - #12863

Merged
dialvarezs merged 4 commits into
nf-core:masterfrom
dialvarezs:semibin-gpu
Sep 3, 2026
Merged

feat(semibin): add GPU support to single_easy_bin and multi_easy_bin#12863
dialvarezs merged 4 commits into
nf-core:masterfrom
dialvarezs:semibin-gpu

Conversation

@dialvarezs

@dialvarezs dialvarezs commented Sep 1, 2026

Copy link
Copy Markdown
Member

Same treatment COMEBin got in #12796, now for SemiBin2.

Both semibin/singleeasybin and semibin/multieasybin pick their container and conda environment from task.accelerator, add the process_gpu label, and pass the resolved device to SemiBin2 with --engine. Both also emit the CUDA runtime version to the versions topic.

The singleeasybin test moves to the shared metagenome binning dataset, with all three BAMs. That meant dropping --environment global: the pretrained model only supports single-sample binning, and it skips the training the GPU is for. Training makes bin names and contents vary across machines, so the snapshot now keeps the stable outputs plus a bin count assertion, as multieasybin already does.

multieasybin moves off delete_me/semibin2/ to binning/multisample/, which has the five samples SemiBin2 requires before it accepts --abundance instead of BAMs. Its contigs use C as the sample separator, Vamb's default, so the tests pass -s C.

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 necessary, include test data in your PR.
  • Remove all TODO statements.
  • Broadcast software version numbers to topic: versions
  • Follow the naming conventions.
  • Follow the parameters requirements.
  • Follow the input/output options guidelines.
  • Add a resource label
  • Use BioConda and BioContainers if possible to fulfil software requirements.
  • nf-core modules test semibin/singleeasybin --profile singularity
  • nf-core modules test semibin/multieasybin --profile singularity

dialvarezs and others added 2 commits September 2, 2026 12:53
Select the container and conda environment from `task.accelerator`, and pass
the resolved device to SemiBin2 with `--engine`. The default is `auto`, which
silently falls back to CPU, so making it explicit keeps the declared
accelerator and the device actually used in agreement. Both modules also emit
the CUDA runtime version to the versions topic, and carry the `process_gpu`
label so the accelerator comes from the pipeline's gpu profile.

The singleeasybin test moves to the shared metagenome binning dataset, with
all three BAMs so the multi-BAM coverage path is exercised. That means
dropping `--environment global`: the pretrained model only supports
single-sample binning, and it skips training altogether, which is the part
the GPU path is about. Bin names and contents are not stable across machines,
so the snapshot follows what multieasybin already does and keeps the stable
outputs plus a bin count assertion.

multieasybin keeps its own test data. `multi_easy_bin` needs a concatenated
FASTA of several assemblies, which the binning dataset does not provide.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`multi_easy_bin` needs one assembly per sample, which the binning dataset did
not provide, so these tests still pointed at `delete_me/semibin2/`. The
dataset now carries that form under `binning/multisample/`, added in
nf-core/test-datasets#2255, and all three tests use it.

Note the new data has five samples, not the three the rest of the binning
dataset uses. SemiBin2 only accepts `--abundance` in place of BAMs from five
samples up, so that is what the abundance test needs, and it also matches what
the old data had.

The contigs use `C` as the sample separator, which is Vamb's default, so the
tests pass `-s C` rather than relying on SemiBin's own default of `:`. That
keeps the data usable by both tools without a colon ending up in the names of
the bins Vamb writes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dialvarezs
dialvarezs force-pushed the semibin-gpu branch 3 times, most recently from 8cf21c4 to 4796650 Compare September 2, 2026 17:34
@dialvarezs
dialvarezs marked this pull request as ready for review September 2, 2026 19:39
The comment said the names and number of bins are not stable, without
saying how that was established. Trying the opposite established it:
snapshotting everything reproduces byte for byte across repeated local runs,
including every bin checksum, and then fails on CI for both modules, on
every profile. Pinning the visible cores and the OpenMP and BLAS thread
counts does not change the local result either, so the drift is not down to
threading and cannot be pinned from a test config.

So the sanitized form stays, and the comments now say the drift is between
machines rather than between runs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dialvarezs
dialvarezs force-pushed the semibin-gpu branch 2 times, most recently from 9e61813 to c86400f Compare September 2, 2026 20:35
@dialvarezs
dialvarezs requested review from a team as code owners September 2, 2026 20:35
With the bins out of the snapshot, the tests only checked that some bins
came out and that the csv and tsv files had the expected names. One thing
holds regardless of how the clustering lands, so it is asserted directly:
the contigs SemiBin featurised in `data.csv` are exactly the contigs
written across the bins, each once. For multi_easy_bin that is checked per
sample, which also covers the sample split itself.

The bins are read with the nft-fasta plugin, which is loaded for the repo
here, so a bin that is not FASTA fails the same assertion. The snapshot
stays for the versions and file names.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@Joon-Klaps Joon-Klaps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely implemented.

@dialvarezs
dialvarezs added this pull request to the merge queue Sep 3, 2026
Merged via the queue into nf-core:master with commit 9f8642c Sep 3, 2026
36 checks passed
@dialvarezs
dialvarezs deleted the semibin-gpu branch September 3, 2026 07:32
@dialvarezs dialvarezs mentioned this pull request Sep 3, 2026
10 tasks
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.

2 participants