Skip to content

bamCompare: compute --operation first/second/add/mean and orient reciprocal_ratio as documented - #1458

Open
cindykrafft wants to merge 1 commit into
deeptools:masterfrom
cindykrafft:fix4/bamcompare-operations
Open

bamCompare: compute --operation first/second/add/mean and orient reciprocal_ratio as documented#1458
cindykrafft wants to merge 1 commit into
deeptools:masterfrom
cindykrafft:fix4/bamcompare-operations

Conversation

@cindykrafft

Copy link
Copy Markdown

Fixes #1457.

calc_ratio (src/calc.rs) matched only log2, ratio, reciprocal_ratio and subtract; its catch-all arm computed the log2 ratio, so the four remaining --operation choices (first, second, add, mean) wrote the log2 track. The reciprocal_ratio arm returned b/a for a/b ≥ 1 and −a/b otherwise, the inverse of the documented "a/b if a/b ≥ 1 else −b/a" that 3.5.x's getRatio implemented (2/3 became −0.67 instead of −1.5). This adds four explicit arms returning the scaled signals without a pseudocount (as before, and as the --pseudocount help says), swaps the two reciprocal_ratio branches, and makes the catch-all a panic so an unknown operation cannot produce a plausible track. Behaviour change to flag in review: the existing test_calc_ratio pinned the inverted reciprocal value (-0.27 for 6/22); it now expects −22/6 = −3.67. Not changed here, but worth a look: the subtract arm adds both pseudocounts before subtracting (invisible at the default --pseudocount 1, a shift of p1 − p2 with two different pseudocounts; 3.5.x subtracted the scaled signals only).

Tests: test_calc_ratio corrected, test_calc_ratio_reciprocal_ratio_sign_and_orientation and test_calc_ratio_first_second_add_mean added (src/tests/test_calc.rs); test_bam_compare_operations_first_second_add_mean_reciprocal checks all five operations on the shipped testA.bam/testB.bam. On master the pytest fails (first writes 3R 0 50 0 | 50 100 -1 | …) and the cargo tests fail (left: -0.27, right: -3.67; left: 0.5, right: 2.0); with the patch cargo test 50 passed, test_bamCoverage_and_bamCompare.py 22 passed, full pytest suite 233 passed.

Found in Mytochondria, a volunteer project that checks the numerical core of research software and verifies every finding by execution (methods and harnesses: https://github.com/cindykrafft/mytochondria/tree/main/audits/deeptools)


Generated by Claude Code


…procal_ratio as documented

calc_ratio matched only log2, ratio, reciprocal_ratio and subtract; its
catch-all arm computed the log2 ratio, so the four remaining choices the
CLI offers (first, second, add, mean) wrote the log2 track.  The
reciprocal_ratio arm returned b/a for a/b >= 1 and -a/b otherwise, the
inverse of the documented "a/b if a/b >= 1 else -b/a" that the 3.5.x
getRatio implemented (2/3 became -0.67 instead of -1.5).

first/second/add/mean output the scaled signals without a pseudocount,
as before (the --pseudocount help: only used with log2 / ratio).  The
catch-all now panics on an unknown operation instead of silently
computing something else.

Tests: the existing test_calc_ratio expectation for reciprocal_ratio
(6/22 -> -22/6 = -3.67) is corrected, two cargo tests are added, and a
pytest checks all five operations on the shipped testA/testB.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TaHntBDKuZJpMAAMenkC44
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.

bamCompare (4.0.0 Rust backend): --operation first/second/add/mean write the log2 ratio, and reciprocal_ratio is inverted

1 participant