Skip to content

CellReads.stats rows in whitelist order, not thread order - #371

Open
BenjaminDEMAILLE wants to merge 1 commit into
mainfrom
fix/cellreads-stats-order
Open

BenjaminDEMAILLE wants to merge 1 commit into
mainfrom
fix/cellreads-stats-order

Conversation

@BenjaminDEMAILLE

Copy link
Copy Markdown
Contributor

Part of #354 (CellReads.stats row order changed with --runThreadN).

Problem

Solo.out/<feature>/CellReads.stats had the same rows at 1, 4 and 8 threads, but in a different order each time. CellReadStats wrote the cell rows in reverse order of first appearance. That order came from whichever thread folded a read into the shared accumulator first, so it depended on scheduling. The module docs and DIVERGENCE.md 3.2 said it was thread-independent; it was not.

What STAR does (2.7.11b source)

  • The per-cell counters live in a std::unordered_map<uintCB, array<uint64,nBits>> (SoloCommon.h:47), filled with insert (SoloCommon.h:60).
  • The map is reserved for nCB*3/2 entries (SoloFeature_countCBgeneUMI.cpp:42), then filled from each thread's temporary file, thread 0 first (SoloFeature_countCBgeneUMI.cpp:45-48, SoloReadFeature_inputRecords.cpp:141-152).
  • The rows are written by walking that map (SoloFeature_statsOutput.cpp:102). CBnotInPasslist comes first (:97-100).

So STAR's order is the standard library's bucket layout, after rehashes, not a sort. The STAR build here (Linux) uses libstdc++. As a check, I fed STAR's 725 keys to a libstdc++ unordered_map in rustar's first-appearance order with reserve(338): 713 of 725 lines landed where STAR has them. The 12 others are likely reads whose first appearance differs between the two tools. The old rustar order (reverse first appearance) matched 1 of 725 cell lines.

Change

  • src/solo/cell_reads.rs: write the cell rows by iterating the BTreeMap that already holds them, keyed by whitelist index. That is whitelist order, sorted by barcode. The order vector and seen set are gone. CBnotInPasslist stays first.
  • Matching STAR's order would mean re-implementing libstdc++'s hash table: its prime bucket list, rehash policy and node chaining. libstdc++ is GPL with a runtime exception, which CONTRIBUTING.md counts as a sensitive licence. So this PR does not do that.
  • DIVERGENCE.md 3.2 rewritten: it wrongly named libc++ and claimed thread-independence. It is marked Needs maintainer sign-off.

Measured against STAR 2.7.11b

data/parity/solo (100k reads, 50k-barcode whitelist, mm19 subset), --soloType CB_UMI_Simple --soloFeatures Gene --outSAMtype None, STAR with --soloCellReadStats Standard, rustar-aligner with --soloCellReadStats CB. Each tool used its own index. STAR is the Linux build in Docker.

Run Lines md5 of file md5 of sorted file First cell rows
STAR, 1 thread 727 419f291d… 73f38551… AAAGTAGCATCCGGGT ACATACGTCTACTCAT
STAR, 4 threads 727 419f291d… 73f38551… same
STAR, 8 threads 727 419f291d… 73f38551… same
main, 1 thread 747 850f503e… b54794b6… AACTCAGTCTGCTGTC AATCGGTCATTGGCGC
main, 4 threads 747 22e85bd9… b54794b6… ACATGGTGTGTTGAGG ACCAGTAAGCTCCTTC
main, 8 threads 747 a6024efb… b54794b6… AATCGGTCATTGGCGC ACAGCCGCAAGCTGAG
this PR, 1 thread 747 0896877e… b54794b6… AAACCTGAGACAAGCC AAACCTGAGCTAGTGG
this PR, 4 threads 747 0896877e… b54794b6… same
this PR, 8 threads 747 0896877e… b54794b6… same

rustar-aligner's file is now the same at every thread count. The sorted md5 is unchanged, so no value moved. Lines at the same position as STAR's: 3 of 727 (header, CBnotInPasslist, one cell row by chance), before and after.

The row values differ from STAR, before and after this PR, and that is not addressed here: rustar-aligner writes 20 cell rows STAR does not, 238 of 726 shared rows differ (mostly exonic, also cbMatch/cbMMmultiple), and CBnotInPasslist counts 30 reads against STAR's 94260.

Tests

rows_follow_whitelist_order_not_arrival_order (replaces rows_are_emitted_in_reverse_first_appearance_order): two arrival orders give the same file, in whitelist order. It fails on main (["CB9", "CB1", "CB4"]) and passes here. umi_and_gene_totals_come_from_the_matrix updated for the new order. cargo test --release, cargo clippy --all-targets -- -D warnings and cargo fmt --check are clean on macOS.

Not checked

  • STAR was identical at 1, 4 and 8 threads on this input. By construction (thread files read in thread order) it could change with thread count on larger inputs; I did not test that.
  • rustar-aligner rejects STAR's --soloCellReadStats Standard and only takes CB. That is a separate gap.
  • The row-value differences listed above.
  • Merges cleanly with Solo: refuse what the read loops cannot do, check barcode and UMI input #360 (git merge-tree), which also touches CellReads.stats code in src/solo/mod.rs and count.rs.

🤖 Generated with Claude Code

The cell rows were written in reverse order of first appearance, taken
from whichever thread folded a read first, so the file changed with
--runThreadN. Write them in whitelist order (sorted by barcode) from the
BTreeMap that already holds them. STAR's own order is a libstdc++
unordered_map walk; reproducing it would mean porting libstdc++'s hash
table, so DIVERGENCE.md 3.2 documents the difference for sign-off.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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