Claude/sparse by default - #2
Open
atuldeshpande wants to merge 4 commits into
Open
Conversation
…,G) builds
prune_transcripts_nuclear_seed: new contract is
`npmi: NpmiBootstrapResult | pd.DataFrame | None = None`.
- None → compute_npmi_bootstrap(df, ...) inline (self-sufficient).
- NpmiBootstrapResult → use directly (the cached / external path).
- pd.DataFrame → auto-converted to a sparse-CSR-backed
NpmiBootstrapResult via the new _pairs_df_to_bootstrap_result
helper. Emits DeprecationWarning; no dense (G,G) ever built.
build_dense_npmi_matrix and the use_sparse_panel branch are gone from
this wrapper; the kernel call is sparse-only. The legacy nan_fill knob
is accepted for back-compat but ignored — the sparse skip semantics win
(an observed PMI of exactly 0.0 stays a stored, counted entry).
aux["W"] is now always a sparse CSR.
Downstream fallout (callers that assumed dense aux["W"]):
- spatial.py: lift three near-identical inner `negative_set` closures
into a single module-level _make_negative_set_lookup helper. The new
helper is sparse-NATIVE: it walks W.getrow(g).data /.indices, no
todense, no per-gene (G,) temp allocation. ~20× lighter than a
densify shim on a typical WT panel (~5% nnz). One of the three call
sites (reassign_unassigned_to_nearest_tx_no_neg) previously lacked
sparse-W support entirely — the consolidation silently fixes that
latent bug too.
- density_cascade.py: temporary symmetrize-then-densify shim when
aux["W"] is sparse — the kernel still does dense W[g,s] indexing.
Eats O(G^2) here for now; sparsifying density_cascade properly is
the next commit.
tests/test_phase1_prune_sparse.py: the wrapper-level skip-vs-zero-fill
discriminator is gone (the wrapper has only one backend now); the
kernel-level test still proves the skip semantics. New
`test_wrapper_dataframe_input_routes_through_sparse_path` asserts the
deprecated DataFrame path matches the bootstrap path and ignores
nan_fill.
Full suite 169 pass; regression refs unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the prior symmetrize-then-densify shim in density_cascade_phase1 with a sparse-CSR-native pair accessor `_pmi_get`. The kernel's only W touchpoint is the scalar `W[gi, gj]` lookup inside `greedy_prune`'s bad- edge counter loop; that now goes through `_pmi_get` (binary-search the symmetric CSR row for sparse, direct index + NaN→0 for dense), so a sparse `aux["W"]` from the prune stays sparse all the way through the cascade — no O(G^2) materialization, no per-call temp arrays. Preserves the existing coherence convention (absent ≡ 0, counted as a bad edge when threshold > 0) — outputs are bit-identical to the legacy densify path. The convention IS opposite to the prune / reassign sparse convention (skip absent); that's a separate conversation, not a silent behavioral shift here. Symmetrization uses the same COO-stack pattern established elsewhere (NOT W + W.T — scipy's sparse add drops explicit zeros) and never eliminate_zeros — an observed PMI of 0.0 stays a stored entry. Closes the last (G,G) allocation along the prune → reassign → density_cascade chain on the production path. Suite 169 pass; refs unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The main-into-feature merge resolution left two kernel-call blocks in
prune_transcripts_nuclear_seed: my pre-merge sparse-only call (without
the new hybrid params) AND the hybrid-aware
`if use_sparse_panel: ... else: ...` block. The `use_sparse_panel` flag
was deleted by my refactor (the wrapper is sparse-only now), so the
hybrid branch errored at runtime:
NameError: name 'use_sparse_panel' is not defined
Fix: keep the veto_mode / hybrid-param parsing block, collapse the
if/else to a single sparse kernel call carrying the new Phase-1b params
(veto_mode_int, min_admit_threshold, mean_admit_threshold,
aggregator_percentile, rs_thr_kernel, neg_npmi_threshold). Delete the
orphaned no-hybrid-params first call. The dense kernel call site is
gone — the wrapper's NpmiBootstrapResult / DataFrame / None contract
always lands on a symmetric CSR via _symmetric_csr_arrays.
Full suite 190 pass (was 169 pre-merge; merge brought in the new
test_phase1b_hybrid_admission tests).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
remove dense PMI dependencies