Skip to content

Better abstraction for keeping track of which constraints have been evaluated - #116

Merged
dance858 merged 1 commit into
mainfrom
problem-jacobian-version-checks
Sep 5, 2026
Merged

Better abstraction for keeping track of which constraints have been evaluated#116
dance858 merged 1 commit into
mainfrom
problem-jacobian-version-checks

Conversation

@dance858

@dance858 dance858 commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Before this PR the problem class kept track on which constraints had to be re-evaluated in each iteration. It is a better abstraction to move this responsibility to the constraints themself. In particular, this is good for the separate Python interface in the future.

Claude below:

Summary

The last of the three values_version follow-ups (after #114 and #115). problem_jacobian kept its own affine shortcut: a problem-global jacobian_called bool, set after the first call, reset wholesale by problem_update_params. This PR replaces it with the counter scheme from #113:

  • problem gains a per-constraint constraint_jac_seen array (allocated and seeded deliberately stale in problem_init_jacobian, freed in free_problem); the jacobian_called field is deleted.
  • problem_jacobian now calls eval_jacobian on every constraint — affine ones return instantly via the impl-skip from affine optimization #114 — and memcpys a constraint's block into the aggregated Jacobian only when that constraint's values_version moved. The offset walk is unchanged (layout is fixed at init), and the assert(nnz_offset == J->nnz) stays.
  • The flag reset in problem_update_params is deleted: expr_set_needs_refresh re-arms the constraint evals, which bump their versions, so the copy re-fires automatically — per constraint, rather than for all constraints at once.

Behavior: affine constraints skip both eval work and the memcpy after their first call, exactly as before but per-constraint and automatic; non-affine constraints bump every eval and copy every call, as before.

One pre-existing note (unchanged by this PR): the memcpy source c->jacobian->x assumes the value-buffer layout matches the init-time to_csr layout — true for sparse_matrix and permuted_dense, but would be wrong if a constraint's top-level Jacobian were ever a stacked_pd (block-major storage).

Tests

New test_problem_jacobian_memcpy_skip: an affine parameterized constraint (a*x) plus a non-affine one (exp(x)). After the first problem_jacobian, the affine slice of prob->jacobian->x is poisoned and a re-eval proves it is not rewritten while the non-affine slice is; a problem_update_params then restores the slice with the new parameter value.

Verification

  • All 426 tests pass (Debug and UBSan builds)
  • Guard-malloc + leaks clean: MallocScribble=1 MallocGuardEdges=1 leaks --atExit — 0 leaks
  • PROFILE_ONLY build compiles
  • clang-format applied; grep confirms no jacobian_called reference remains

problem_jacobian used a problem-global bool: first call evaluates all
constraints, later calls skip affine ones entirely, and
problem_update_params resets the flag wholesale. Replace it with a
per-constraint seen counter against the constraint jacobian's
values_version: every constraint is eval'd (affine ones return instantly
via the impl-skip from #114), and a block is memcpy'd into the aggregated
jacobian only when its version moved. Re-copying after a parameter update
happens automatically because expr_set_needs_refresh re-arms the evals,
which bump the versions - so the copy skip is per-constraint and needs no
reset.

The seen array is seeded deliberately stale in problem_init_jacobian so
the first call always copies. New poison-based regression test proves the
affine slice is not rewritten on re-eval and is restored after a
parameter update.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016rQ3LPdi31kNLmTy9F2oEa
@dance858 dance858 changed the title Replace jacobian_called flag with per-constraint version checks [WIP] Replace jacobian_called flag with per-constraint version checks Aug 31, 2026
@dance858 dance858 changed the title [WIP] Replace jacobian_called flag with per-constraint version checks Better abstraction for keeping track of which constraints have been evaluated Sep 5, 2026
@dance858
dance858 merged commit 497d3a4 into main Sep 5, 2026
12 checks passed
@dance858
dance858 deleted the problem-jacobian-version-checks branch September 5, 2026 15:24
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