fix: ft call-peaks --haps fills the H1/H2 columns - #141
Merged
Merged
Conversation
The flag was parsed on CallPeaksOptions but call_peaks_for_chrom hardcoded haps: false, so the pileup never built haplotype tracks and every H1/H2 column was zero. Move haps into PeakCallingParams so it reaches the pileup. union-peaks sets it false since BED intervals carry no HP tag. Closes #140
mrvollger
added a commit
that referenced
this pull request
Sep 18, 2026
Fixes #140. `ft call-peaks --haps` printed zero for every H1/H2 column on a haplotagged BAM, while `ft pileup --haps` on the same BAM was correct. ## Cause `--haps` was parsed on `CallPeaksOptions`, but `call_peaks` hardcoded `haps: false` when it built the pileup. The pileup never made H1/H2 tracks, so every peak printed the empty-track placeholder (`0 0 -1.0 0 0`) for both haplotypes. ## Fix Pass `opts.haps` through. One line. ## Check `NAPA.bam` carries HP tags. `ft call-peaks --haps --min-fire-frac 0.5` on it: | | coverage | coverage_H1 | coverage_H2 | |---|---|---|---| | before | 95 | 0 | 0 | | after | 95 | 45 | 14 | A regression test pins these three values. ## Patch release (0.13.1) Targets `release/v0.13` so it ships in 0.13.1. Forward-port to main is #141. Co-authored-by: Mitchell R. Vollger <mvollger@gmail.com>
mrvollger
changed the title
Only all_data's fire_elements are read (peak boundaries in call-peaks). With --haps now live, the H1/H2 tracks were allocating a Vec per base that nothing used, about a third of the tripled call-peaks memory.
mrvollger
added a commit
that referenced
this pull request
Sep 18, 2026
Twin of #148 for main, rebased onto #147 so the two do not conflict on the release-plz.toml comment. Merging this brings in #147's commit too; #147 can then be closed as included. Same content as #148 plus the fix from #150: `semver_check = false` lives in the `[workspace]` table. **Do not merge until #132 (the 0.13.1 release PR on `release/v0.13`) is merged.** release-plz finds its open release PR by branch prefix only. A push to main with the semver check off would run release-plz-pr, find #132 by its `release-plz-` branch name, and force-push 0.14.0 content onto it. Today the only thing preventing that is main's run dying in the semver check, which this PR removes. After #132: merge this first on main, then #141 and #138. release-plz then opens a fresh 0.14.0 release PR. --------- Co-authored-by: Mitchell R. Vollger <mvollger@gmail.com>
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Forward-port to main of the 0.13.1 fix for #140, which shipped from
release/v0.13in #142.Cause
--hapswas parsed onCallPeaksOptions, butcall_peaks_for_chromhardcodedhaps: falsewhen it built the pileup. The pileup never made H1/H2 tracks, so every peak printed the empty-track placeholder (0 0 -1.0 0 0) for both haplotypes.Fix
hapsmoves fromCallPeaksOptionsintoPeakCallingParams, which is whatcall_peaks_for_chromreceives. Both structs are flattened, so the flag name and help text are unchanged;--hapsnow appears among the peak-calling options in--helpinstead of last. union-peaks sets it false because BED intervals carry no HP tag.The second commit keeps
--hapsfrom costing more than it must. Per-haplotype tracks triple the pileup memory per chromosome, and about a third of that was FIRE element tracking on the H1/H2 tracks that nothing reads (onlyall_data.fire_elementsfeeds peak boundaries). The haplotype tracks are now built without it.Check
NAPA.bamcarries HP tags.ft call-peaks --haps --min-fire-frac 0.5on it:A regression test pins these three values. The call-peaks snapshot and pileup tests pass.
Order
Merge into main after #149 and before #138. release-plz opens the 0.14.0 release PR on the first push to main after #149.
Fixes #140