problem_jacobian: copy constraint values through the CSR view - #123
Merged
Merged
Conversation
problem_init_jacobian lays out the aggregated Jacobian from each constraint's to_csr view, but problem_jacobian copied values from the constraint matrix's own buffer. For a stacked_pd that buffer is block-major, which equals CSR row order only when the blocks cover contiguous row ranges; a constraint like transpose(A @ X) with a multi-column X has interleaved blocks and its rows were copied in the wrong order. Copy from the to_csr view instead. Sparse and permuted_dense views alias their buffer, so nothing changes for them; stacked_pd refreshes its cache once per changed evaluation under the existing version guard. Regression test builds exactly that constraint and checks the assembled Jacobian row by row; it fails on main with rows 1 and 2 swapped. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B6Bs765i1HTbqx3LUu6LMP
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.
Summary
Bug fix found while planning lesson 3 M3; independent of that work.
problem_init_jacobianlays out the aggregated constraint Jacobian from each constraint'sto_csrview, butproblem_jacobiancopied values from the constraint matrix's raw->x. For a stacked_pd that buffer is block-major, which coincides with CSR row order only when the blocks cover contiguous row ranges. A constraint such astranspose(left_matmul_dense(A 2x3, X 3x2))has blocks on rows {0,2} and {1,3}, so rows 1 and 2 of the problem Jacobian were swapped.Fix: copy from
c->jacobian->to_csr(c->jacobian)->x. Sparse returns its own CSR and permuted_dense's view aliases its buffer, so nothing changes for them; stacked_pd refreshes its CSR cache once per changed evaluation under the existingcsr_seenversion guard. Theconstraint_jac_seenskip logic is unchanged.Test
test_problem_jacobian_spd_constraint_interleavedbuilds that constraint, asserts the constraint Jacobian is stacked_pd, and checks the assembled Jacobian row by row against the dense expectation. Fails on main (actual[3] = 4.0, expected 1.0in row 1), passes with the fix.451 tests Release, 453 with SP_TRACK_MEMORY. UBSan, guard-malloc + leaks, clang-format clean.
🤖 Generated with Claude Code
https://claude.ai/code/session_01B6Bs765i1HTbqx3LUu6LMP