Skip to content

Add a row_reduce primitive and migrate sum onto it (lesson 3, M3) - #124

Open
dance858 wants to merge 4 commits into
problem-jacobian-to-csrfrom
row-reduce
Open

dance858 wants to merge 4 commits into
problem-jacobian-to-csrfrom
row-reduce

Conversation

@dance858

Copy link
Copy Markdown
Collaborator

Summary

Lesson 3, milestone 3. Stacked on #123 (the problem_jacobian fix), which its problem-level test depends on; retarget to main once #123 merges.

  • New vtable pair row_reduce_alloc(A, group, m_out) / row_reduce_fill_values(A, C): C[j, :] is the sum of rows i of A with group[i] == j. The input-nnz to output-position map is bound to the result (bound_iwork), so the fill takes no map. Fills zero the output and accumulate, never bump values_version (the eval wrapper does), and are safe under the affine skip.
  • sum builds group from its axis (0, i / d1, i % d1 over the child's column-major rows) and calls the primitive. sum_expr.idx_map and free_type_data are gone.
  • Kernels: sparse builds each output row's sorted column union with a marker array (same shape as the old block-sum helper, generalised); permuted_dense keeps col_perm and collapses the hit groups into a sorted row_perm; stacked_pd reduces per block and goes through the existing coalesce-accumulate skeleton, keeping the raw spd on pre_coalesce.
  • Intended behaviour change: sum of a stacked_pd child is stacked_pd for every axis. Previously axis -1 gave a 1-row pd and axes 0/1 fell back to CSR. Trade-off: the merged block takes the column union per row signature, so non-kron-shaped spd children can carry structural zeros where the CSR fallback had exact nnz (same trade-off as transpose_spd).
  • Deleted: sum_row_partition_alloc (slot, typedef, doc), sparse_sum_row_partition_alloc, sum_all_rows/sum_block_of_rows/sum_evenly_spaced_rows_csr_alloc, the three pd helpers plus dispatcher, the spd fallback, and two dead includes in stacked_pd.c. accumulator, the trace helpers, sum_4_csr_alloc, compose_csr_idx_map_for_spd and sorted_union_int_arrays stay (other callers).
  • tests/profiling/profile_log_reg.h: the manual "Path B" baseline now uses the primitive on the PD (note: it times the dense reduce rather than the CSR accumulator). Values still match Path A to 0.

Tests

  • New tests/utils/test_row_reduce.h: sparse (column merge, empty output row, NaN-poisoned fill proving the memset, refill), pd (non-monotone collapsing group, bound_iwork, twin match), spd cross-block accumulate on a shared column, spd within-block routing, spd all-to-one, and an alloc-free spd fill under SP_TRACK_MEMORY.
  • Atom level: sum of a 2-block spd child for axes -1/0/1 asserting is_stacked_pd plus numerical Jacobian check; axis 1 also checked exactly through the CSR view.
  • Problem level: sum(A @ X, axis=1) as a constraint reaches the aggregated Jacobian in row order.
  • Removed the three pd unit tests that inspected the caller's idx_map.

457 tests Release, 460 with SP_TRACK_MEMORY. UBSan, guard-malloc + leaks, PROFILE_ONLY (built and run), clang-format clean. Mutation checks, each rebuilt in isolation: dropping the sparse memset, reading the wrong pd source row, plain scatter instead of accumulate in the spd fill, and swapping the atom's axis formulas all fail the suite.

🤖 Generated with Claude Code

https://claude.ai/code/session_01B6Bs765i1HTbqx3LUu6LMP

dance858 and others added 4 commits September 12, 2026 15:45
row_reduce_alloc(A, group, m_out) / row_reduce_fill_values(A, C) replace the
sum_row_partition_alloc slot: C[j, :] = sum of rows i with group[i] == j. The
input-nnz to output-position map lives on the result (bound_iwork), so the
sum atom no longer owns an idx_map sized by the child's nnz. One generic
kernel per kind replaces the three axis-specific ones: sparse builds the
per-row column union with a marker array; permuted_dense keeps col_perm and
collapses hit groups into a sorted row_perm; stacked_pd reduces per block and
coalesce-accumulates, so sum of a stacked_pd child is now stacked_pd for every
axis instead of a 1-row pd (axis -1) or a CSR fallback (axes 0/1).

Deleted: the slot, sparse_sum_row_partition_alloc, the three CSR row-sum
helpers, the three pd helpers and dispatcher, the spd fallback, and
sum_expr.idx_map. The profile_log_reg baseline is ported to the primitive.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B6Bs765i1HTbqx3LUu6LMP
sum_all_rows_csr_alloc, sum_block_of_rows_csr_alloc and
sum_evenly_spaced_rows_csr_alloc are standalone CSR operations worth keeping
for a sparse linear algebra library even though the engine no longer calls
them; tile_int likewise. Moved verbatim into the old-code folder, which is
still compiled.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B6Bs765i1HTbqx3LUu6LMP
…+ col_to_pos

Positions below the current row start mean "not in this row yet", so the
same n-long array serves as the membership marker and, after sorting, as the
column-to-position map. Saves one n-long transient at init.

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