Skip to content

Remove use of C variables starting in _ from bam_consensus.c.#2250

Merged
vasudeva8 merged 2 commits into
samtools:developfrom
jkbonfield:consensus-C-namespace
Aug 14, 2025
Merged

Remove use of C variables starting in _ from bam_consensus.c.#2250
vasudeva8 merged 2 commits into
samtools:developfrom
jkbonfield:consensus-C-namespace

Conversation

@jkbonfield

Copy link
Copy Markdown
Contributor

This conflicts with _P definition in newlib's ctype.h in cygwin.

Fixes #2248

Comment thread bam_consensus.c Outdated
This conflicts with _P definition in newlib's ctype.h in cygwin.

Fixes samtools#2248
@jkbonfield jkbonfield force-pushed the consensus-C-namespace branch from e378562 to e2b6b15 Compare August 13, 2025 09:01
The defaule Bayesian method does a lot of local modification of
mapping quality via the local mismatch count.  This is floating point
based, but the variable being integer means lots of unnecessary type
conversions happen.  It's surprising this is such a significant
performance cost compared to everything else that goes on (including a
division which can be avoided using lookup tables, but isn't
significant).
@jkbonfield

Copy link
Copy Markdown
Contributor Author

I also added another tiny commit which isn't technically related to the original issue, but I don't feel like creating multiple PRs and then someone having to deal with resolving any conflicts plus having to do all the testing and evaluation twice.

For local users, my assessment was on this file:

samtools coolensus -r LG08:1-1000000 ~jkb/scratch/data/Female_pacbio.bam -o /dev/nul

The md5sum consensus didn't differ, but the -f pileup output did due to subtle quality value changes, due to avoiding repeated rounding conversions.

@vasudeva8 vasudeva8 merged commit 3952c89 into samtools:develop Aug 14, 2025
6 checks passed
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.

Makefile bam_consensus.o Error

3 participants