Skip to content

Dupblaster insertion#62

Open
MattWellie wants to merge 31 commits into
mainfrom
dupblaster_retry
Open

Dupblaster insertion#62
MattWellie wants to merge 31 commits into
mainfrom
dupblaster_retry

Conversation

@MattWellie

@MattWellie MattWellie commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Restart of #61

Purpose

Proposed Changes

  • Inserts DupBlaster as an in-stream tool, as a Picard alternative
  • Updates the Dockerfile to install an updated samtools and dupblaster alongside the workflow
  • 2 new tools are installed conservatively, using a compilation layer to reduce final foorprint
  • Inserts new methods to name-sort data (used in single-alignment case, and between merging and dupblasting), and coordinate sort explicitly - used after dupblasting

Depends on

Checklist

  • Related GitHub Issue created
  • Tests covering new change
  • Linting checks pass

@MattWellie MattWellie requested review from EddieLF and Johnnyassaf July 6, 2026 03:19
Comment thread src/align_genotype/jobs/align.py Outdated
storage_gb = storage_for_align_job(alignment_input)
nthreads = config.config_retrieve(['workflow', 'align_threads'], 16)
job.storage(f'{storage_gb}GiB')
job.cpu(nthreads)

@Johnnyassaf Johnnyassaf Jul 9, 2026

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 like the explicit aspect here, but be careful the machine setting isnt interfering with anything here. This would affect nthreads and permissons to use job.memory maybe

in the rnaseq jobs i set them like this:

from cpg_flow.resources import HIGHMEM
HIGHMEM.set_resources(j=j, ncpu=nthreads, storage_gb=50)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

That's why I've dropped down to explicit resourcing - there's a contradiction in CPG-Flow's resource profiling.

Line 1: "GCP is one thread per vCPU"
Method: get ncpu by starting with threads, dividing by two, and getting the nearest power of two

Based on Google's documentation there are 2 threads available per CPU, so the fixed value of 1 is incorrect, and affects calculations.

This conflict was meaning that if I request a 16 core VM manually, and I run with 16 threads it's fine. If I try to use all the helper methods I request 16 threads, it's converted to a 8 CPU machine, and the profile gives me 8 threads instead.

I CBA using the helper methods which work and ignoring the ones which don't. It made sense when the code contained a decision about whether to use a highmem profile or a standard one, but it's a memory & cpu intensive process, so always using highmem feels defensible.

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.

This is quite literally a backbone of the pipeline so I agree that if anything deserves highmem it'll be here

from hailtop.batch.job import BashJob


def markdup(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Love to see it

Comment thread src/align_genotype/config_template.toml Outdated
samtools = "australia-southeast1-docker.pkg.dev/cpg-common/images/samtools:1.23.1-1"
somalier = "australia-southeast1-docker.pkg.dev/cpg-common/images/somalier:0.3.1-1"
verifybamid = "australia-southeast1-docker.pkg.dev/cpg-common/images/verifybamid:2.0.1"
verifybamid = "australia-southeast1-docker.pkg.dev/cpg-common/images/verifybamid:v2.0.3-1"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hate this v

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It hates u 2

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.

Bin off MarkDuplicates

3 participants