Skip to content

Fix add's Hessian when its children share a subexpression - #159

Merged
dance858 merged 1 commit into
mainfrom
add-shared-hessian
Oct 13, 2026
Merged

dance858 merged 1 commit into
mainfrom
add-shared-hessian

Conversation

@dance858

Copy link
Copy Markdown
Collaborator

Summary

  • Bug: add evaluated both children's Hessians and then summed them. When the children share a node, evaluating the right child re-evaluates that node with different weights. That overwrites Hessian values the left child still needed before the sum reads them. add(log x, exp(log x)) gave -2 where -4 is correct.
  • Fix: the Hessian sum is now filled incrementally with sum_n_matrices_fill_zero / sum_n_matrices_fill_add, so each child's Hessian is added right after it is evaluated. The pattern comes from sum_n_matrices_alloc with n = 2, and add still needs no struct of its own.
  • Jacobians: the Jacobian path is unchanged. A shared node's Jacobian doesn't depend on weights, so re-evaluating it writes the same values.
  • Pin: sparsediff_linalg moves to cba21ac (library Adds indexing atom implementation and other bindings #20, which adds fill_zero / fill_add).

Other atoms that evaluate several children and sum afterwards (multiply/matmul via sum_4, problem.c's Lagrange Hessian) likely have the same issue with shared nodes. They aren't changed here.

Tests

  • New test_wsum_hess_add_shared_subexpr checks add(log x, exp(log x)) and add(exp(log x), log x) against the numerical Hessian. Both use a single shared log x node.
  • The test fails on the old add.c (analytical -2, numerical -4) and passes with the fix.
  • All 301 tests pass in Debug and Release.

🤖 Generated with Claude Code

add evaluated both children's Hessians and then summed them. When the
children share a node, evaluating the right child re-evaluates that node with
other weights and overwrites values the left child's Hessian still needed, so
add(log x, exp(log x)) gave -2 instead of -4. Now the sum is filled
incrementally with sum_n_matrices_fill_zero / fill_add, adding each child's
Hessian right after it is evaluated.

Pins sparsediff_linalg to cba21ac (#20, which adds fill_zero / fill_add).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dance858
dance858 merged commit de2160f into main Oct 13, 2026
14 checks passed
@dance858
dance858 deleted the add-shared-hessian branch October 15, 2026 05:39
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