Refresh composite param_source subtrees on parameter updates#107
Merged
Conversation
Transurgeon
force-pushed
the
param-source-mark-refresh
branch
from
July 12, 2026 22:16
c93a83c to
4248f7d
Compare
…them A param_source subtree is evaluated by the owning atom's gated recursion, not the main forward walk, so expr_set_needs_refresh never reaches nodes inside a composite source. Gated nodes there (promote, nested mults, cached matmul coefficients) kept serving values cached at build time after problem_update_params: p*A @ x and quad_form(x, g*Sig) both solved with the original parameter values on every re-solve. Fix: each source-owning atom marks its subtree dirty at the moment it is about to re-evaluate it (one expr_set_needs_refresh call per atom, gated on the flag the main walker already sets). Nested gated levels recurse for free. No struct or walker changes; no behavior change for one-shot solves or bare-parameter sources. Tests: tests/problem/test_param_source_refresh.h pins the three failing shapes (composite matmul coefficient, composite quad matrix, two nested gated levels); each fails without the fix and passes with it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Transurgeon
force-pushed
the
param-source-mark-refresh
branch
from
July 12, 2026 22:24
4248f7d to
fa911a7
Compare
This was referenced Jul 13, 2026
Closed
Open
Collaborator
|
Beautiful solution. I could stare at it for hours. |
Transurgeon
added a commit
that referenced
this pull request
Jul 13, 2026
…them (#107) A param_source subtree is evaluated by the owning atom's gated recursion, not the main forward walk, so expr_set_needs_refresh never reaches nodes inside a composite source. Gated nodes there (promote, nested mults, cached matmul coefficients) kept serving values cached at build time after problem_update_params: p*A @ x and quad_form(x, g*Sig) both solved with the original parameter values on every re-solve. Fix: each source-owning atom marks its subtree dirty at the moment it is about to re-evaluate it (one expr_set_needs_refresh call per atom, gated on the flag the main walker already sets). Nested gated levels recurse for free. No struct or walker changes; no behavior change for one-shot solves or bare-parameter sources. Tests: tests/problem/test_param_source_refresh.h pins the three failing shapes (composite matmul coefficient, composite quad matrix, two nested gated levels); each fails without the fix and passes with it. Co-authored-by: Transurgeon <peter.zijie@gmail.com> Co-authored-by: Claude Fable 5 <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
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.
Problem
A
param_sourcesubtree is evaluated by the owning atom's gated recursion, not the main forward walk, soexpr_set_needs_refreshnever reaches nodes inside a composite source. Gated nodes there —promote, nested mults, cached matmul coefficients — keep serving values cached at construction afterproblem_update_params.The failing shapes are common, not exotic (each verified with a standalone repro):
vector_mult(promote(p), A)feedingleft_matmul(p * A) @ xvector_mult(promote(g), Sig)feedingquad_formquad_form(x, g * Sig)quad_formquad_over_lin(x, p)(canonicalized toI/p)((2*p) * A) @ xShapes that were already correct and stay untouched: bare-
Parametersources (refreshed byproblem_update_params' memcpy), purely additive composites (p1+p2— ungated nodes recurse unconditionally), and computed multipliers of variables ((2*p)*x— the gated node sits in the main tree, which the walker reaches).Fix
One line per source-owning atom (
scalar_mult,vector_mult,left_matmul— which also servesright_matmul—convolve,kron,quad_form): mark the side subtree dirty at the moment the atom is about to re-evaluate it,The owner's flag is already set by the existing walker (it lives in the main tree), so the mark happens exactly once per parameter update; nested gated levels recurse for free because each level applies the same line to its own source. No struct changes, no walker changes, no behavior change for one-shot solves or bare-parameter sources.
Tests
tests/problem/test_param_source_refresh.hpins the three failing shapes (composite matmul coefficient, composite quad matrix, two nested gated levels), asserting constraint values and Jacobian/gradient values across an update. Each fails without the fix (e.g. stale10.0where50.0is expected) and passes with it. Full suite: 417/417.Downstream validation (cvxpy
pr-a/b/cstack, cached re-solve paths): the cvxpy-side canaries (quad_over_linsymbolic quad matrix, scaled parameter coefficient) and all four repro shapes above go from stale to fresh on this branch with no other changes; full cvxpy test suite green (modulo pre-existing, unrelated failures verified identical on other builds).Relation to #104
This supersedes the first commit of #104 (param-source refresh propagation) with a smaller, atom-local change: no
param_sourcepointer hoisted into the baseexprstruct. #104's second commit (retaining registered parameter nodes) is deliberately not included — it is an independent API-soundness hardening that can be its own PR if wanted.🤖 Generated with Claude Code