Perf T3-3: Deduplicate K symbol flattening#139
Open
hmzh-khn wants to merge 4 commits into
Open
Conversation
Tests that setup_approximate_kkt_solver returns all_K_syms_vec in its named tuple, avoiding redundant computation in preoptimize_nonlinear_solver. TDD Red step: test fails because setup_info doesn't yet include all_K_syms_vec. Co-Authored-By: Claude Opus 4.6 <[email protected]>
setup_approximate_kkt_solver now returns all_K_syms_vec in its named tuple. preoptimize_nonlinear_solver reuses this instead of recomputing the same vcat+reshape expression. This eliminates one redundant symbolic vector flattening during solver construction. The third occurrence (line 695) operates on numerical K_evals at solve time, not symbolic K_syms during setup. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Co-Authored-By: Claude Opus 4.6 <[email protected]>
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
2 participants
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
Eliminates redundant K symbol vector flattening during solver construction. The same
vcat([reshape(something(K_syms[ii], ...), :) for ii in 1:N]...)expression was computed identically in bothsetup_approximate_kkt_solverandpreoptimize_nonlinear_solver. Now computed once and passed through the setup_info named tuple.Changes
src/nonlinear_kkt.jl: Addedall_K_syms_vecto the named tuple returned bysetup_approximate_kkt_solver; replaced duplicate computation inpreoptimize_nonlinear_solverwithsetup_info.all_K_syms_vectest/test_k_sym_dedup.jl: New test verifying the deduplication (5 assertions: field exists, length match, symbolic equality, augmented variable consistency)test/test_tiers.jl: Registered new test in fast tiertest/test_test_tiers.jl: Updated expected file listsRETROSPECTIVES.md: Added retrospective entryImpact
Minor — eliminates one redundant symbolic vector concatenation during setup. Not a hot-path change (setup runs once per solver construction).
Testing
test_k_sym_dedup.jladded to fast tierChangelog
🤖 Generated with Claude Code