Repository navigation
Center dense data inside the sandwich kernel - #560
Open
Marc-Antoine Schmidt (MarcAntoineSchmidtQC) wants to merge 5 commits into
Open
Marc-Antoine Schmidt (MarcAntoineSchmidtQC) wants to merge 5 commits into
Marc-Antoine Schmidt (MarcAntoineSchmidtQC) wants to merge 5 commits into
Conversation
StandardizedMatrix.sandwich expanded the product into uncentered terms that cancel when a column's mean is large relative to its std. For a dense matrix, the sandwich kernel now subtracts the column means while packing its panels, so the product is computed on centered data without copying it. Also fix StandardizedMatrix.astype dropping mult.
1 task done
For a SplitMatrix, the dense columns are now centered in all kernels that read them: the dense sandwich, the sparse x dense and categorical x dense cross products, and the dense column sums. Their part of the shift then vanishes, so the remaining expansion only involves the sparse and categorical shifts.
StandardizedMatrix.matvec and transpose_matvec now center the dense columns inside the kernels, like sandwich, instead of adding the shift to the uncentered product. glum's gradient and the intercept row of its Hessian go through transpose_matvec. The dense matrix-vector kernels take an optional center and vectorize along the direction that is not restricted, so that the centered products keep about the speed of BLAS; small products run serially.
- SplitMatrix.sandwich, matvec and transpose_matvec share one loop with their centered versions, and the cross blocks with the dense part go through the usual _cross_sandwich dispatch with a dense_center argument. - MatrixBase has default _centered_* methods, so StandardizedMatrix calls them on any matrix; _dense_centering centers column by column and always returns (center, shift). - The dense kernels always get a center (zeros if none), validated by one Cython helper; the cross kernels keep a null center for their uncentered path, which would otherwise be slower. - Fix transpose_matvec returning the columns in their original order when cols lists all of them in a different order.
Marc-Antoine Schmidt (MarcAntoineSchmidtQC)
marked this pull request as ready for review
November 5, 2026 19:18
Marc-Antoine Schmidt (MarcAntoineSchmidtQC)
requested review from
Jan Tilly (jtilly) and
Martin Stancsics (stanmart)
as code owners
November 5, 2026 19:18
Marc-Antoine Schmidt (MarcAntoineSchmidtQC)
requested a balanced review from Copilot
November 5, 2026 19:19
Copilot started reviewing on behalf of
Marc-Antoine Schmidt (MarcAntoineSchmidtQC)
November 5, 2026 19:19
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Active correctness bugs discard meaningful shift residuals, break products on standardized dense slices, and return columns in the wrong order for unsigned indexers.
4 open issues
What changed in this PR
Improves numerical stability of standardized matrix products by centering dense data inside the kernels, addressing #414 without copying the matrix.
Changes:
- Adds in-kernel centering for dense products and sparse/categorical cross-products.
- Vectorizes dense matrix-vector kernels and fixes scaling preservation and permuted-column ordering.
- Adds numerical-accuracy and regression tests, plus release notes.
| File | Description |
|---|---|
tests/test_standardized_mat.py |
Tests accuracy, slicing, casting, and column ordering. |
tests/test_fast_sandwich.py |
Tests centered kernels across layouts and restrictions. |
src/tabmat/standardized_mat.py |
Tracks centers and dispatches centered products. |
src/tabmat/split_matrix.py |
Routes centers through dense blocks and cross-products. |
src/tabmat/sparse_matrix.py |
Passes dense centers into sparse cross-products. |
src/tabmat/matrix_base.py |
Adds centered-product hooks and preserves exact means. |
src/tabmat/ext/split.pyx |
Exposes centering for categorical–dense kernels. |
src/tabmat/ext/sparse.pyx |
Exposes centering for sparse–dense kernels. |
src/tabmat/ext/sparse_helpers-tmpl.cpp |
Centers dense values during cross-product packing. |
src/tabmat/ext/dense.pyx |
Validates centers and dispatches dense kernels. |
src/tabmat/ext/dense_helpers-tmpl.cpp |
Implements centering and vectorized matrix-vector operations. |
src/tabmat/ext/cat_split_helpers-tmpl.cpp |
Centers dense values in categorical cross-products. |
src/tabmat/dense_matrix.py |
Adds centered-product dispatch and column-order handling. |
src/tabmat/categorical_matrix.py |
Forwards centers to categorical–dense kernels. |
CHANGELOG.rst |
Documents accuracy fixes and performance changes. |
Review effort: Balanced
- transpose_matvec compares cols with the identity directly; np.diff wraps around for unsigned indices, so permuted uint32 cols counted as unrestricted. - Slicing a StandardizedMatrix squeezes the center like shift and mult, so the (1, k) column indexer of np.ix_ no longer leaves it two-dimensional.
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.

Fixes #414, as an alternative to #540 that does not copy the data.
StandardizedMatrixcomputes its products by expandingX * mult + shiftinto uncentered terms. These cancel when a column's mean is large relative to its std, which can lose all significant digits. This PR subtracts the column means inside the C++ kernels while they read the dense data, so the products are computed on centered data without a copy:sandwich: the dense sandwich kernel centers while packing its panels. For aSplitMatrix, the sparse × dense and categorical × dense cross kernels do the same.matvec/transpose_matvec: the dense matrix-vector kernels take a center and are vectorized along the unrestricted direction, so restricted products also get faster.standardize()keeps the exact means. For aStandardizedMatrixbuilt directly, they are recovered fromshift / mult.Also fixes
StandardizedMatrix.astypedroppingmult, andtranspose_matvecignoring the order ofcolswhen it lists all columns in a different order.Benchmarks
float64 on an M1 Pro, against
mainand #540 withmaterialize_shift=True.Accuracy at mean/std = 1e6 (max error / max |value|, against data centered in float64):
sandwichtranspose_matvecsandwichtranspose_matvecStandardizedMatrixbuilt directly,sandwichSpeed (ms) and memory, 1M×50 unless noted:
standardizesandwich, F / C ordermatvec, F ordertranspose_matvec, F / C ordermatvecwithcols, 100k×500sandwich(50 dense, 20 sparse, 2 categoricals)#540 is faster for unrestricted
matvec/transpose_matvecon F-order data, because it can call BLAS on its copy. On C-order data, OpenBLAS'sX.T @ vis slow; this PR's kernel avoids it.glum fits (time / iterations / coefficient error against a fit on centered data):
irls-lsirls-cdirls-ls#540 only helps if glum passes the flag; this PR fixes glum's current call. The reproducer from Quantco/glum#872 converges in 12 IRLS iterations (0.9 s) instead of hitting the 100-iteration cap (300 s). glum's test suite, including the golden masters, passes against this branch.
Checklist
CHANGELOG.rstentry