Skip to content

Fix HISAT2 issues: BAM CSI index with UMI deduplication #1644

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
pinin4fjords wants to merge 16 commits into from
Closed

Fix HISAT2 issues: BAM CSI index with UMI deduplication #1644

pinin4fjords wants to merge 16 commits into from

Conversation

@pinin4fjords
Copy link
Member

@pinin4fjords pinin4fjords commented Dec 2, 2025

Summary

  • Fix BAM CSI index access error when using --with_umi and --bam_csi_index true with HISAT2 (and STAR) aligners
  • The BAM_DEDUP_UMI subworkflow already handles CSI/BAI logic internally and emits through its bai output channel
  • The main workflow was incorrectly trying to access a non-existent .out.csi channel

Related

Changes

The fix changes:

# Before (broken)
ch_genome_bam_index = params.bam_csi_index ? BAM_DEDUP_UMI_HISAT2.out.csi : BAM_DEDUP_UMI_HISAT2.out.bai

# After (fixed)
ch_genome_bam_index = BAM_DEDUP_UMI_HISAT2.out.bai

Same fix applied for both STAR and HISAT2 aligners.

Test plan

  • Test with --aligner hisat2 --with_umi --bam_csi_index true
  • Test with --aligner star_salmon --with_umi --bam_csi_index true

🤖 Generated with Claude Code

pinin4fjords and others added 16 commits November 27, 2025 11:14
@pinin4fjords @claude
Bump version after release 3.22.0
1b7e091 
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@pinin4fjords @claude
Update CHANGELOG with PR number
7ec73dd 
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@mashehu
add arm CI test
4f1b8b0
@mashehu
trigger a test (to be reverted)
0ee4547
@pinin4fjords
Merge pull request #1640 from nf-core/bump/post-release-3.22.0
e8fce2d 
Bump after release 3.22.0
@mashehu
fix name in CI
5515c2a
@pinin4fjords @claude
Add ARM CI support with SKIP_SENTIEON
5b5481a 
- Add SKIP_SENTIEON environment variable support to nf-test.config
  for ARM runners and fork CI runs where Sentieon is unavailable
- Split Sentieon tests into separate files (main.sentieon.nf.test)
  for selective exclusion via nf-test ignore patterns
- Update CI workflows to set SKIP_SENTIEON on ARM and forks

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@pinin4fjords @claude
Update containers for ARM compatibility
77af5d2 
- Update STAR igenomes modules to Wave containers with samtools 1.21
  for consistent versions across x86 and ARM architectures
- Add ARM container selectors for STAR_ALIGN_IGENOMES and
  STAR_GENOMEGENERATE_IGENOMES modules
- Update DESEQ2_QC container configuration for ARM support
- Update arm.config with ARM-compatible container mappings

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@pinin4fjords @claude
Update tests and snapshots for ARM compatibility
17ff6e9 
- Remove hardcoded --platform=linux/amd64 from test nextflow configs
- Update DESEQ2_QC tests for ARM architecture compatibility
- Update test snapshots for new container versions (samtools 1.21)
- Fix arriba test to snapshot BAM filename only to avoid
  architecture-specific content differences
- Remove sentieon test entries from main snapshot files
  (now in separate main.sentieon.nf.test.snap files)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@mashehu
test it with all profiles
52ffcee
@mashehu
Revert "test it with all profiles"
6a784d7 
This reverts commit 52ffcee.
@mashehu
Revert "trigger a test (to be reverted)"
a7f657b 
This reverts commit 0ee4547.
@mashehu
update changelog
52ff9ad
@mashehu
Merge branch 'dev' into add-arm-ci-tests
441cf47
@mashehu
Merge pull request #1641 from mashehu/add-arm-ci-tests
692b6a0
@pinin4fjords @claude
Fix BAM CSI index access with UMI deduplication
6a5bdda 
The BAM_DEDUP_UMI subworkflow handles CSI/BAI index logic internally
and emits the appropriate index through its `bai` output channel.
The main workflow was incorrectly trying to access a non-existent
`csi` output channel when `bam_csi_index` was true.

Fixes the error: "No such property: csi for class:
groovyx.gpars.dataflow.DataflowBroadcast"

This fix applies to both STAR and HISAT2 aligners when used with
--with_umi and --bam_csi_index options.

Fixes #1643

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@github-actions
Copy link

github-actions bot commented Dec 2, 2025

This PR is against the master branch ❌

  • Do not close this PR
  • Click Edit and change the base to dev
  • This CI test will remain failed until you push a new commit

Hi @pinin4fjords,

It looks like this pull-request is has been made against the nf-core/rnaseq master branch.
The master branch on nf-core repositories should always contain code from the latest release.
Because of this, PRs to master are only allowed if they come from the nf-core/rnaseq dev branch.

You do not need to close this PR, you can change the target branch to dev by clicking the "Edit" button at the top of this page.
Note that even after this, the test will continue to show as failing until you push a new commit.

Thanks again for your contribution!

@github-actions
Copy link

github-actions bot commented Dec 2, 2025 •
edited
Loading

nf-core pipelines lint overall result: Failed ❌

Posted for pipeline commit 6a5bdda

+| ✅ 286 tests passed       |+
#| ❔   8 tests were ignored |#
!| ❗   9 tests had warnings |!
-| ❌   3 tests failed       |-

❌ Test failures:

❗ Test warnings:

  • files_exist - File not found: assets/multiqc_config.yml
  • pipeline_todos - TODO string in base.config: Check the defaults for all processes
  • pipeline_todos - TODO string in awsfulltest.yml: You can customise AWS full pipeline tests as required
  • pipeline_todos - TODO string in methods_description_template.yml: #Update the HTML below to your preferred methods description, e.g. add publication citation for this pipeline
  • pipeline_todos - TODO string in main.nf: Optionally add in-text citation tools to this list.
  • pipeline_todos - TODO string in main.nf: Optionally add bibliographic entries to this list.
  • pipeline_todos - TODO string in main.nf: Only uncomment below if logic in toolCitationText/toolBibliographyText has been filled!
  • pipeline_todos - TODO string in nextflow.config: Specify any additional parameters here
  • pipeline_if_empty_null - ifEmpty(null) found in /home/runner/work/rnaseq/rnaseq/subworkflows/local/prepare_genome/main.nf: _ versions = ch_versions.ifEmpty(null) // channel: [ versions.yml ]
    _

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 3.5.1
  • Run at 2025-12-02 11:21:08

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

possible tiny bug with bam_csi_index & hisat2 aligner & umi

3 participants

@pinin4fjords @mashehu