Skip to content

Add blobtk depth for read coverage#171

Merged
sainsachiko merged 31 commits into
devfrom
hhoang-read-coverage
Jul 16, 2025
Merged

Add blobtk depth for read coverage#171
sainsachiko merged 31 commits into
devfrom
hhoang-read-coverage

Conversation

@sainsachiko

Copy link
Copy Markdown
Contributor
  • Add blobtk depth for read coverage

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 you've added a new tool - have you followed the pipeline conventions in the contribution docs
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

@sainsachiko sainsachiko requested a review from muffato June 10, 2025 10:41
@sanger-tolsoft

Copy link
Copy Markdown
Contributor

Warning

Newer version of the nf-core template is available.

Your pipeline is using an old version of the nf-core template: 3.2.1.
Please update your pipeline to the latest version.

For more documentation on how to update your pipeline, please see the nf-core documentation and Synchronisation documentation.

Comment thread conf/base.config
Comment thread modules/local/blobtk_depth.nf Outdated
Comment thread modules/local/blobtk_depth.nf Outdated
Comment thread docs/output.md Outdated
Comment thread docs/output.md Outdated
sainsachiko and others added 3 commits June 20, 2025 15:32
- Remove cpus limit as blobtk depth is single-threaded
- Adjust coverage file name
- Fix typos

Co-authored-by: Matthieu Muffato <mm49@sanger.ac.uk>

@muffato muffato left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you update the CHANGELOG too ?

I can reproduce the issue with -resume you were saying this morning. Did you find out why ? If you run Nextflow with -dump-hashes you can find out what changes between runs, cf https://www.nextflow.io/docs/latest/cache-and-resume.html#comparing-the-hashes-of-two-runs

Comment thread conf/base.config Outdated
@muffato muffato mentioned this pull request Jun 24, 2025
9 tasks
sainsachiko and others added 15 commits June 27, 2025 14:13
- `process_*` don't exist in this pipeline. The defaults resources are already for single CPU use
- No need to invoke a container, the host must have `ln`
- Therefore no need to report versions
- CHANGE_NAME is always called and is the only place where the published
  file name is set
- All downstream modules use the renamed BAMs as input and copy its name
  to the output
We don't publish anything from CHANGE_NAME itself. Instead we make
SAMTOOLS_INDEX publish its input file.
@muffato muffato mentioned this pull request Jul 3, 2025
9 tasks
@muffato muffato force-pushed the hhoang-read-coverage branch from 5034665 to 9e02bf0 Compare July 3, 2025 16:05
@muffato muffato mentioned this pull request Jul 3, 2025
9 tasks
@muffato muffato self-requested a review July 15, 2025 09:48

@muffato muffato left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I've just found there's an issue with the BAM output !
When running this branch with the --outfmt bam option, the output (BAM) files only contain the header, not any read alignment.

@muffato

muffato commented Jul 15, 2025

Copy link
Copy Markdown
Member

And another issue is that pacbio samples (i.e. mMelMel3_T3 and mMelMel3_T4) go through CONVERT_STATS twice. They may be going twice earlier than that but I'm not sure. I may be confused by the chunking.

$ grep CONVERT_STATS:SAMTOOLS_FLAGSTAT results/pipeline_info/execution_trace_2025-07-15_10-54-16.txt | cut -f4 | awk '{print $2}' | sort | uniq -c
      1 (mMelMel1_T1)
      1 (mMelMel2_T1)
      1 (mMelMel3_T1)
      1 (mMelMel3_T2)
      2 (mMelMel3_T3)
      2 (mMelMel3_T4)

@muffato

muffato commented Jul 15, 2025

Copy link
Copy Markdown
Member

I've run some tests (CRAM/BAT/both, and with/without header). No issue about empty BAM file like this morning.

In BAM + no-header mode, the BAM files are not published (but the BAI are)

results_coverage8_noh_bam/read_mapping/hic
results_coverage8_noh_bam/read_mapping/hic/GCA_922984935.2.subset.unmasked.hic.mMelMel3_T1.flagstat
results_coverage8_noh_bam/read_mapping/hic/GCA_922984935.2.subset.unmasked.hic.mMelMel3_T1.coverage.bedGraph.gz
results_coverage8_noh_bam/read_mapping/hic/GCA_922984935.2.subset.unmasked.hic.mMelMel3_T1.stats
results_coverage8_noh_bam/read_mapping/hic/GCA_922984935.2.subset.unmasked.hic.mMelMel3_T1.idxstats
results_coverage8_noh_bam/read_mapping/hic/GCA_922984935.2.subset.unmasked.hic.mMelMel3_T1.bam.bai

(to enable the no-header mode, I edit conf/test.config and comment out the line)

@sainsachiko sainsachiko merged commit 5e363cf into dev Jul 16, 2025
4 of 7 checks passed
@sainsachiko sainsachiko deleted the hhoang-read-coverage branch July 16, 2025 10:45
@muffato muffato added this to the v1.4.0 milestone Feb 19, 2026
@muffato muffato mentioned this pull request Feb 20, 2026
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.

3 participants