Skip to content

funannotate train: operator-precedence bug in bamthreads calculation (aux_scripts/trinity.py) #1179

Description

@hyphaltip

Describe the bug

In runTrinityGG(), the number of samtools sort compression threads is
computed as:

https://github.com/nextgenusfs/funannotate/blob/target_1.9/rust_EVM_trinity_PASA/funannotate/aux_scripts/trinity.py#L54

bamthreads = (args.cpus + 2 // 2) // 2

Python operator precedence evaluates 2 // 2 first (= 1), so this is
actually (args.cpus + 1) // 2, not (args.cpus + 2) // 2 as the spacing
implies was intended (a "use half the CPUs, rounded up" comment would fit
(args.cpus + 1) // 2 for round-up-of-half, but the + 2 visually suggests
a different rounding intent that isn't what's executed).

Concretely:

Not a functional break either way, just not computing what the code visually
appears to intend, and worth fixing while touching nearby throughput-tuning
code.

Suggested fix

Either add explicit parens to compute the evidently-intended
(args.cpus + 2) // 2, or simplify to the clearer max(1, args.cpus // 2)
if that's the actual intent (leave headroom for the hisat2/pipe side of the
hisat2 | samtools sort pipeline). Whichever is chosen should be a
deliberate one-line fix, not a silent behavior change disguised as a
formatting cleanup.

What command did you issue?

funannotate train -i genome.fa --left_norm R1 --right_norm R2 --cpus N ...
(any invocation that reaches runTrinityGG()'s hisat2/samtools alignment
step)

OS/Install Information

Found via static code review; not tied to a specific OS.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions