Skip to content

Saturate --max-idepth instead of overflowing when scaled by sample count - #2617

Open
clintval wants to merge 1 commit into
samtools:developfrom
clintval:fix/max-idepth-overflow
Open

clintval wants to merge 1 commit into
samtools:developfrom
clintval:fix/max-idepth-overflow

Conversation

@clintval

@clintval clintval commented Nov 3, 2026 •

Copy link
Copy Markdown

As of right now, mpileup scales the per-sample --max-idepth by the number of samples in an int.

With a large value and an even number of samples the product can become negative! This results in total_depth < max_indel_depth becoming never true, and indel calling is then silently skipped at every site.

I hit this calling variants per cell on single-cell data, where each cell is its own read group in one BAM (14,382 samples). For example, -L 2147483647 dropped every indel, while the default of -L 250 retained the indels.

The fix in this PR multiplies in 64 bits and clamps to INT_MAX.

This PR also updates the usage text to describe -L as a per-sample depth, matching the manual.

@clintval
clintval force-pushed the fix/max-idepth-overflow branch 3 times, most recently from eb57e24 to cfd53ea Compare November 3, 2026 06:48
The per-sample --max-idepth is multiplied by the number of samples in an int, so a large value wraps negative with an even sample count and silently disables indel calling. Multiply in 64 bits and clamp to INT_MAX, and describe -L as a per-sample depth in the usage text, as the manual already does.

Signed-off-by: Clint Valentine <valentine.clint@gmail.com>
@clintval
clintval force-pushed the fix/max-idepth-overflow branch from cfd53ea to 8b4ebe2 Compare November 3, 2026 16:43

This branch has not been deployed

No deployments
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.

1 participant