Implement SNP distance models with gamma correction and PHYLIP output
Renames the CLI flag from --metric to --distance and introduces eight closed-form SNP distance models with optional Jin-Nei gamma correction. Integrates the ndarray crate for matrix operations and adds relaxed PHYLIP output formatting. Updates architecture and theory documentation to cover the new sparse matrix variants, algorithmic fixes, and distance metric implementations.
This commit is contained in:
Eric Coissac
@@ -1367,6 +1367,17 @@
|
||||
</span>
|
||||
</a>
|
||||
|
||||
</li>
|
||||
|
||||
<li class="md-nav__item">
|
||||
<a href="#persistentcompactintmatrixsparse-implemented-2026-08-26" class="md-nav__link">
|
||||
<span class="md-ellipsis">
|
||||
|
||||
PersistentCompactIntMatrix::Sparse — implemented (2026-08-26)
|
||||
|
||||
</span>
|
||||
</a>
|
||||
|
||||
</li>
|
||||
|
||||
</ul>
|
||||
@@ -1652,6 +1663,17 @@
|
||||
</span>
|
||||
</a>
|
||||
|
||||
</li>
|
||||
|
||||
<li class="md-nav__item">
|
||||
<a href="#persistentcompactintmatrixsparse-implemented-2026-08-26" class="md-nav__link">
|
||||
<span class="md-ellipsis">
|
||||
|
||||
PersistentCompactIntMatrix::Sparse — implemented (2026-08-26)
|
||||
|
||||
</span>
|
||||
</a>
|
||||
|
||||
</li>
|
||||
|
||||
</ul>
|
||||
@@ -2582,6 +2604,98 @@ mismatches. The dense/sparse performance gap is gone — previously sparse
|
||||
systematic gap. <code>pack --sparse</code>'s claimed query win isn't confirmed
|
||||
outright by this (sparse should arguably now <em>beat</em> dense on truly sparse
|
||||
real data, not just tie), but the pathological regression is fixed.</p>
|
||||
<h2 id="persistentcompactintmatrixsparse-implemented-2026-08-26"><code>PersistentCompactIntMatrix::Sparse</code> — implemented (2026-08-26)</h2>
|
||||
<p>Closes the gap flagged throughout this document ("no sparse count format
|
||||
exists yet", <code>traits.rs:9-12</code>'s "Explicitly deferred"): <code>obicompactvec</code>
|
||||
already had <code>PersistentSparseCompactIntMatrix</code> (row-major, built on top of
|
||||
<code>PersistentSparseBitMatrix</code> as its "which columns are non-zero" support,
|
||||
values <em>not</em> deduplicated — see that struct's own doc comment), but it was
|
||||
never wired into <code>PersistentCompactIntMatrix</code>, the dense-dispatching enum
|
||||
every real consumer (<code>TypedLayer<PersistentCompactIntMatrix></code>,
|
||||
<code>KmerLayer::Count</code>) actually holds. Concretely: <code>kmer_index.rs::
|
||||
pack_matrices(sparse=true)</code> already called <code>pack_sparse_compact_int_matrix</code>
|
||||
on every layer's <code>counts/</code> — but <code>PersistentCompactIntMatrix::open</code> had no
|
||||
code path back to what that just wrote, so a <code>Count</code> layer became
|
||||
unreadable ("no count matrix found ... run 'obikmer upgrade'") the moment
|
||||
anyone ran <code>pack --sparse</code> on an index with count layers. Root cause, not a
|
||||
workaround: add the missing <code>Sparse</code> variant.</p>
|
||||
<ul>
|
||||
<li>
|
||||
<p><strong>Enum + dispatch</strong> (<code>intmatrix.rs</code>): <code>PersistentCompactIntMatrix::Sparse
|
||||
(PersistentSparseCompactIntMatrix)</code>, detected in <code>open</code>/<code>detect_storage</code>
|
||||
via a <code>singleton_values.pciv</code> marker (mirrors <code>PersistentBitMatrix</code>'s own
|
||||
<code>sparse_meta.json</code> check), reported via <code>storage_kind()</code>. <code>col</code>/
|
||||
<code>col_view</code>/<code>col_persist</code> panic/<code>Unsupported</code> on <code>Sparse</code>, same convention
|
||||
as the bit side. <code>sub_matrix</code>/<code>fill_sub_matrix</code> and <code>nonzero_iter</code>
|
||||
unified the same way <code>PersistentBitMatrix</code>'s already are (drain
|
||||
<code>nonzero_iter</code>, one traversal per format — see "Implemented
|
||||
(2026-08-20)" above); <code>nonzero_iter</code> had to become <code>Box<dyn Iterator<...>></code>
|
||||
for the same reason (<code>Columnar</code>/<code>Packed</code>/<code>Sparse</code> are different concrete
|
||||
types). No change needed in <code>obikindex</code> at all — <code>KmerLayer::Count</code>
|
||||
already only ever holds <code>TypedLayer<PersistentCompactIntMatrix></code>, so the
|
||||
enum absorbing <code>Sparse</code> fixes the unreadable-layer bug for free, same as
|
||||
<code>PersistentBitMatrix::Sparse</code> already did on the presence side.</p>
|
||||
</li>
|
||||
<li>
|
||||
<p><strong><code>CountPartials</code>, non-naive</strong> (<code>sparse_intmatrix.rs</code>): unlike
|
||||
<code>PersistentSparseBitMatrix</code>'s dict-driven <code>col_weights_and_pair_counts</code>,
|
||||
values here aren't deduplicated (two rows can share the same non-zero
|
||||
column set via the same <code>dict_id</code> while carrying different counts), so
|
||||
the "weight by how many rows share a dict entry" shortcut doesn't carry
|
||||
over. What does: a single row-major pass (<code>row_major_pairwise</code>, decodes
|
||||
each row once via <code>for_each_cell_in_row</code>, nests over that row's own
|
||||
co-present columns) — <code>O(Σ k̄²)</code> over populated rows instead of the naive
|
||||
<code>O(n_cols² × n)</code> column-pair rescan, same complexity class as the bit
|
||||
side minus the dict multiplicity discount. Kernels used: <code>min(a,b)</code>
|
||||
(bray, relfreq-bray — both vanish when either side is absent, so no
|
||||
correction needed), <code>a·b</code> and <code>√(a·b)</code> (euclidean/relfreq-euclidean and
|
||||
hellinger — these <em>do</em> need a correction, reconstructed from per-column
|
||||
marginals via <code>Σ(a-b)² = Σa²+Σb²-2Σab</code>, since <code>(a-0)² = a² ≠ 0</code> unlike
|
||||
the <code>min</code>-based formulas). <code>threshold_jaccard(1)</code> shortcuts straight to
|
||||
<code>support</code>'s own <code>BitPartials::partial_jaccard</code> (threshold 1 is exactly
|
||||
presence); <code>threshold_jaccard(0)</code> is closed-form (every <code>u32</code> is <code>≥ 0</code>).</p>
|
||||
</li>
|
||||
<li>
|
||||
<p><strong>Two pre-existing bugs found and fixed while wiring the <code>threshold==1</code>
|
||||
shortcut</strong> (<code>bitmatrix/sparse.rs</code>, <code>BitPartials for
|
||||
PersistentSparseBitMatrix</code>, present since the 2026-08-15 implementation
|
||||
above, never caught because no test compared <code>Sparse</code>'s raw <code>partial_*</code>
|
||||
output against dense on real data — only the diagonal-blind
|
||||
<code>jaccard_dist_matrix</code>/<code>hamming_dist_matrix</code> finalisations were tested):</p>
|
||||
</li>
|
||||
<li><code>partial_jaccard</code>'s diagonal was <code>(0, 2×col_weights[i])</code> instead of a
|
||||
genuine self-comparison <code>(col_weights[i], col_weights[i])</code> —
|
||||
<code>col_weights_and_pair_counts</code>'s <code>inter</code> never pairs a column with
|
||||
itself by construction.</li>
|
||||
<li><code>partial_hamming</code>'s off-diagonal formula itself was wrong: <code>total -
|
||||
union</code> (count of rows where <em>neither</em> column is present) instead of
|
||||
the actual Hamming distance <code>col_weights[i] + col_weights[j] -
|
||||
2×inter[i,j]</code> (symmetric-difference size). Only coincides with the
|
||||
correct value when <code>col_weights[i] + col_weights[j] == total</code>, so
|
||||
small/synthetic test data could easily have hidden it.</li>
|
||||
</ul>
|
||||
<p>Neither surfaced through <code>jaccard_dist_matrix</code>/<code>hamming_dist_matrix</code>
|
||||
(both explicitly zero their own diagonal at finalisation, and the
|
||||
off-diagonal <code>partial_hamming</code> bug had gone untested against dense
|
||||
entirely) — only visible to a caller of the raw <code>partial_*</code> methods
|
||||
directly, which is exactly what <code>partial_threshold_jaccard(1)</code>'s new
|
||||
shortcut became. Fixed at the source, not patched around at the call
|
||||
site; regression test added:
|
||||
<code>tests::sparse::partial_jaccard_and_hamming_match_dense_including_diagonal</code>.</p>
|
||||
<ul>
|
||||
<li><strong>Tests</strong>: <code>tests::intmatrix::sparse_roundtrip_matches_columnar</code>/
|
||||
<code>sparse_roundtrip_from_packed</code> (the <code>open</code>-dispatch fix, both build
|
||||
paths); <code>tests::intmatrix::sparse_count_partials_match_dense</code> (all six
|
||||
<code>CountPartials</code> formulas, thresholds 0/1/2/3, against <code>Columnar</code> on
|
||||
asymmetric-presence data — this is what caught the diagonal gap in the
|
||||
int side's own new code before it shipped, the same way it exposed the
|
||||
two pre-existing bit-side bugs above); <code>obikindex</code>'s
|
||||
<code>count_layer_transparently_reads_sparse_after_pack</code> — the actual
|
||||
end-to-end regression test for the original "layer unreadable after
|
||||
<code>pack --sparse</code>" bug, built → packed sparse → reopened, compared against
|
||||
the pre-pack dense read. <code>cargo test -p obicompactvec -p obikindex</code>:
|
||||
green, no regressions (180 + 12 tests).</li>
|
||||
</ul>
|
||||
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user