Skip to content

Add snpclustering subworkflow - #11879

Open
dbaku42 wants to merge 59 commits into
nf-core:masterfrom
dbaku42:add-snpclustering-subworkflow
Open

Add snpclustering subworkflow#11879
dbaku42 wants to merge 59 commits into
nf-core:masterfrom
dbaku42:add-snpclustering-subworkflow

Conversation

@dbaku42

@dbaku42 dbaku42 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Description

This PR adds the snpclustering subworkflow for end-to-end unsupervised clustering of genomic
samples from genotyped multi-sample VCF files.

This is a revised and extended version of the subworkflow originally proposed in #11059, which was
based on FlashPCA2 (not available as an nf-core module). The current implementation replaces it
with official nf-core modules throughout and extends the pipeline with imputation, PCA-based
clustering, quality metrics, and visualization steps.

Note: This subworkflow depends on the modules added in #11372
(custom/pcaclustering, custom/clustermetrics, custom/clustervisualization),
which should be merged first.

Pipeline

VCF → BEAGLE5_BEAGLE → PLINK2_VCF → PLINK2_PCA → EIGENVEC_TO_TSV
                                                         ↓
                                          CUSTOM_PCACLUSTERING
                                         ↙                    ↘
                          CUSTOM_CLUSTERMETRICS    CUSTOM_CLUSTERVISUALIZATION

Features

  • Genotype imputation and phasing with beagle5/beagle (reference panel and genetic map optional)
  • Conversion to PLINK2 pgen format with plink2/vcf
  • PCA with plink2/pca
  • Inline .eigenvec → TSV conversion via an embedded EIGENVEC_TO_TSV process (gawk)
  • Unsupervised clustering (k-means, DBSCAN) with custom/pcaclustering
  • Cluster quality metrics with custom/clustermetrics
  • UMAP/t-SNE visualization with custom/clustervisualization

Related PRs

@dbaku42
dbaku42 enabled auto-merge June 5, 2026 10:56
@dbaku42 dbaku42 self-assigned this Jun 12, 2026
@dbaku42
dbaku42 requested review from famosab and removed request for abhi18av, afombravo and agshaarrawi July 6, 2026 10:12
Comment thread subworkflows/nf-core/snpclustering/tests/nextflow.config Outdated
Comment thread subworkflows/nf-core/snpclustering/main.nf Outdated
@dbaku42

dbaku42 commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

thank you @famosab,
Is anything else to fix?

@dbaku42
dbaku42 requested a review from famosab July 20, 2026 11:53
@dbaku42

dbaku42 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Hi @famosab,
I tried to pin down the unstable md5s from the subworkflow test config. Snapshots now hash the TSV, cluster assignments, clustering_info.json and metrics.tsv.
Does this config look acceptable to you?

@dbaku42 dbaku42 removed their assignment Aug 27, 2026
@dbaku42
dbaku42 requested a review from LouisLeNezet August 27, 2026 12:33
dbaku42 and others added 6 commits August 27, 2026 15:23
Drop env/cpu overrides and default ext.args. Pin BEAGLE/PLINK2 seeds
for stable hashes and set GAWK_EIGENVEC_TO_TSV ext.suffix=tsv so the
output does not collide with PLINK2 ${meta.id}.eigenvec.
@dbaku42

dbaku42 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

@famosab a short update on the test/config cleanup, then a question on merge timing.

tests/nextflow.config now only sets what the spec allows plus one required ext key:

  • ext.args seeds on BEAGLE5 / PLINK2_VCF / PLINK2_PCA so hashed PCA/cluster files stay stable
  • ext.suffix = 'tsv' on GAWK_EIGENVEC_TO_TSV so the gawk output does not collide with ${meta.id}.eigenvec (the module asserts this)

Removed from that file: env {}, cpus/memory, and ext.args that only repeated Python defaults.

The eigenvec → TSV conversion (OFS, # strip, IIDsample_id) lives in awk/eigenvec_to_tsv.awk so it is not test-only.

Snapshots no longer md5 non-deterministic files. We still assert content where it is stable, following the custom/pcaclustering module test:

On topics: the three custom modules already emit path "versions.yml", emit: versions, topic: versions on master. This subworkflow does not mix .out.versions, so it is born on topics. #12771 was closed for that reason.

Happy to change anything you still want touched. When you have a moment, is there anything left before this can be merged.

@dbaku42
dbaku42 requested a review from famosab September 1, 2026 11:30
Comment thread subworkflows/nf-core/snpclustering/main.nf Outdated
Comment thread subworkflows/nf-core/snpclustering/main.nf Outdated
Comment thread subworkflows/nf-core/snpclustering/meta.yml
* column to sample_id. Output keeps its default naming (<meta.id>.eigenvec)
* — only the tab-separated content matters downstream, not the extension.
*/
GAWK_EIGENVEC_TO_TSV(

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.

Personally, I wonder if having a dedicated process to remove # and rename IID to sample_id is necessary, especially considering that the recipient of this is a custom module, which could handle this trivial conversion if it is required?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fair question. I left it as a dedicated step so custom/pcaclustering stays generic (any TSV of sample × features) and does not absorb PLINK-specific quirks (# header, IID/FID). The rename to sample_id also matches what clustermetrics / clustervisualization already expect. I’d rather keep that adapter at the subworkflow boundary than special-case PLINK inside the clustering module.

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.

I think it would be better to special-case PLINK format in your clustering module, since it seems to commonly consume files that were generated in PLINK format. Which in my view would actually make the module more generic?

I understand that this is a custom module, but to me it would facilitate wider adoption if this module was tailored to the inputs, and it is a very simple operation (e.g. sed 's/#//; s/IID/sample_id/' "${plinkfile}" > "${tsvfile}" should do it), and I assume that you could simply test if it is necessary based on extension of the input file.

My main concern here is that I assume spawning all of these jobs creates some overhead, and the jobs do very little. Since @famosab has been involved more than I am, maybe there are considerations here that I am not aware of.

dbaku42 and others added 2 commits September 3, 2026 08:49
Co-authored-by: Niklas Schandry <schandry@gmail.com>
Co-authored-by: Niklas Schandry <schandry@gmail.com>
@dbaku42

dbaku42 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

@nschan thanks — this is now in place.

custom/pcaclustering (#12904) and custom/clustermetrics /
custom/clustervisualization (#12906) read PLINK2 .eigenvec
(#FID IID / #IID) directly. This subworkflow no longer runs
GAWK_EIGENVEC_TO_TSV; PLINK2_PCA.out.evecfile goes straight
into clustering, metrics and visualization.

@nschan

nschan commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

custom/pcaclustering (#12904) and custom/clustermetrics / custom/clustervisualization (#12906) read PLINK2 .eigenvec (#FID IID / #IID) directly. This subworkflow no longer runs GAWK_EIGENVEC_TO_TSV; PLINK2_PCA.out.evecfile goes straight into clustering, metrics and visualization.

Great, thanks for taking up that comment and incorporating it. In my view it improves the modules, and streamlines this workflow. I have no further comments. Since I was not involved in this from the beginning, maybe it would be good if @SPPearce and/or @famosab could take another look?

@dbaku42

dbaku42 commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

Hi @famosab, @SPPearce,
Is there anything else to improve?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants