Repository navigation
Conversation
… ints Motivation: top_segment_proportions_sparse_csr allocated the per-cell sums and the running accumulator with the input dtype. For small signed integer sparse data (e.g. int8/int16) these overflow, giving wrong (e.g. negative) pct_counts_in_top_N_genes. The dense path upcasts and is unaffected. The issue also reports a numba typing failure for unsigned dtypes; that did not reproduce here (uint16/uint32 passed even before the change). Approach: allocate sums and acc as float64 (values already is). Validation: added test_top_segments_sparse_small_int, comparing the sparse result to the dense result for int8/int16/uint8/uint16/uint32. With the source change stashed it fails for int8, int16 and uint8; with it, all pass. Ran `pytest tests/test_qc_metrics.py` (61 passed, 7 xfailed) and `ruff check` / `ruff format --check` on changed files. Did not run the rest of CI. Report: scverse#4440 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Assisted-by: claude-sonnet-5-5 (via Claude Code)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4461 +/- ##
=======================================
Coverage 84.37% 84.37%
=======================================
Files 134 134
Lines 12750 12750
=======================================
Hits 10758 10758
Misses 1992 1992
Flags with carried forward coverage won't be shown. Click here to find out more.
|
This branch has not been deployed
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
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.
LLM usage
LLM or coding agent used: Claude Sonnet 5.5 via Claude Code
If one was used, I, a human, have reviewed, understood and tested every changed line
Closes calculate_qc_metrics gives negative pct_counts_in_top_N_genes for sparse int16 X and fails for sparse unsigned integer X #4440
Tests included or not required because: added
test_top_segments_sparse_small_int(fails on int8/int16/uint8 before the fix, passes after)Release notes not necessary because: n/a,
docs/release-notes/4440.fix.mdaddedSums and accumulator in
top_segment_proportions_sparse_csrare now float64 instead of the input dtype, fixing overflow (negative percentages) for small-int sparse data. The numba error for unsigned dtypes from the issue did not reproduce locally (numba version here), so only the overflow is demonstrated.AI assistance: this change was drafted with Claude Code.