Address review comments on parameter-support-v2

- Remove redundant refresh_param_values calls from eval_jacobian and
  eval_wsum_hess in left_matmul (forward always runs first)
- Use memcpy in problem_register_params for pointer array copy
- Add PARAM_FIXED guard in problem_update_params to skip fixed constants
- Remove unused right_matmul_expr struct from subexpr.h
- Add test_param_fixed_skip_in_update covering mixed fixed/updatable params
- Add CLAUDE.md for Claude Code guidance

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: Transurgeon

CLAUDE.md

@@ -0,0 +1,82 @@
+# CLAUDE.md
+
+This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
+
+## Project Overview
+
+SparseDiffEngine is a C library implementing the differentiation engine for CVXPY's DNLP extension. It computes sparse Jacobians and Hessians for nonlinear programming solvers. Used as a backend via Python bindings in the parent SparseDiffPy package.
+
+## Build & Test Commands
+
+```bash
+# Build standalone (CMake)
+mkdir -p build && cd build && cmake .. -DCMAKE_BUILD_TYPE=Debug && make
+
+# Run all tests
+cd build && ctest --output-on-failure
+
+# Run tests directly (more verbose output)
+cd build && ./all_tests
+
+# Rebuild after changes (from build/)
+make && ctest --output-on-failure
+
+# Format code
+clang-format -i src/**/*.c include/**/*.h
+```
+
+**Platform dependencies**: macOS uses Accelerate framework (automatic), Linux needs `libopenblas-dev`, Windows uses OpenBLAS via vcpkg.
+
+## Architecture
+
+### Expression DAG with Function-Pointer Polymorphism
+
+Every node is an `expr` struct (`include/expr.h`) containing function pointers that act as a vtable:
+- `forward()` — evaluate node given variable values `u`
+- `jacobian_init()` / `eval_jacobian()` — sparsity pattern allocation and numeric fill
+- `wsum_hess_init()` / `eval_wsum_hess()` — weighted-sum Hessian
+- `is_affine()` — used for caching optimizations
+- `free_type_data()` — type-specific cleanup
+
+Type-specific nodes use C struct inheritance: embed `expr base` as the first field, then cast. Defined in `include/subexpr.h` (e.g., `parameter_expr`, `scalar_mult_expr`).
+
+All node constructors call `init_expr()` to wire up the dispatch table. Children are owned via `left`/`right` pointers with reference counting (`expr_retain`/`free_expr`).
+
+### Parameter System
+
+`parameter_expr` (in `subexpr.h`) unifies constants and updatable parameters:
+- `param_id == PARAM_FIXED (-1)`: fixed constant, not updatable
+- `param_id >= 0`: offset into the parameter vector `theta`
+
+Bivariate ops that involve parameters (scalar_mult, vector_mult, left_matmul, right_matmul) store a `param_source` pointer and read values during forward/jacobian/hessian evaluation.
+
+**Workflow**: `new_parameter()` → `problem_register_params()` → `problem_update_params(prob, theta)` copies from `theta` into each parameter node's `value` array using `param_id` as offset.
+
+### Problem Interface (`include/problem.h`)
+
+The `problem` struct owns an objective, constraints array, and parameter nodes. Key lifecycle:
+1. `new_problem()` — allocates problem, output arrays
+2. `problem_register_params()` — registers parameter nodes
+3. `problem_init_derivatives()` — allocates sparsity patterns (Jacobian + Hessian)
+4. `problem_objective_forward()` / `problem_constraint_forward()` — evaluate
+5. `problem_gradient()` / `problem_jacobian()` / `problem_hessian()` — compute derivatives
+6. `problem_update_params()` — update parameter values, reset caches
+
+### Sparse Matrix Formats (`include/utils/`)
+
+CSR is primary. CSC is intermediate during computation. COO for Python interop; lower-triangular COO for symmetric Hessians.
+
+## Adding a New Operation
+
+1. Create `src/<category>/new_op.c` implementing the function pointer interface
+2. Add constructor declaration to the appropriate `include/<category>.h`
+3. Write test as a `.h` file in `tests/<test_category>/`
+4. Register test in `tests/all_tests.c` with `mu_run_test(test_name, tests_run)`
+
+## Test Framework
+
+Uses minunit.h (header-only). Tests are `const char *test_name(void)` functions returning NULL on success. Tolerance: `ABS_TOL=1e-6, REL_TOL=1e-6` via `cmp_double_array()` in `test_helpers.c`. No built-in test filtering — all tests run via single `all_tests` binary.
+
+## C Code Style
+
+C99, Allman braces, 4-space indent, 85-column limit, right-aligned pointers (`int *ptr`). Enforced by `.clang-format`. Strict warnings: `-Wall -Wextra -Wpedantic -Wshadow -Wformat=2 -Wcast-qual -Wcast-align -Wdouble-promotion -Wnull-dereference`.

include/subexpr.h

@@ -130,17 +130,6 @@ typedef struct left_matmul_expr
     void (*refresh_param_values)(struct left_matmul_expr *);
 } left_matmul_expr;
 
-/* Right matrix multiplication: y = f(x) * A where f(x) is an expression.
- * f(x) has shape p x n, A has shape n x q, output y has shape p x q.
- * Uses vec(y) = B * vec(f(x)) where B = A^T kron I_p. */
-typedef struct right_matmul_expr
-{
-    expr base;
-    CSR_Matrix *B;  /* B = A^T kron I_p */
-    CSR_Matrix *BT; /* B^T for backpropagating Hessian weights */
-    CSC_Matrix *CSC_work;
-} right_matmul_expr;
-
 /* Scalar multiplication: y = a * child where a comes from param_source */
 typedef struct scalar_mult_expr
 {

src/bivariate/left_matmul.c

@@ -122,7 +122,6 @@ static void jacobian_init(expr *node)
 static void eval_jacobian(expr *node)
 {
     left_matmul_expr *lnode = (left_matmul_expr *) node;
-    refresh_param_values(lnode);
     expr *x = node->left;
 
     CSC_Matrix *Jchild_CSC = lnode->Jchild_CSC;
@@ -157,7 +156,6 @@ static void wsum_hess_init(expr *node)
 static void eval_wsum_hess(expr *node, const double *w)
 {
     left_matmul_expr *lnode = (left_matmul_expr *) node;
-    refresh_param_values(lnode);
 
     /* compute A^T w*/
     Matrix *AT = lnode->AT;

src/problem.c

@@ -335,11 +335,11 @@ void problem_register_params(problem *prob, expr **param_nodes, int n_param_node
 {
     prob->n_param_nodes = n_param_nodes;
     prob->param_nodes = (expr **) malloc(n_param_nodes * sizeof(expr *));
-    prob->total_parameter_size = 0;
+    memcpy(prob->param_nodes, param_nodes, n_param_nodes * sizeof(expr *));
 
+    prob->total_parameter_size = 0;
     for (int i = 0; i < n_param_nodes; i++)
     {
-        prob->param_nodes[i] = param_nodes[i];
         prob->total_parameter_size += param_nodes[i]->size;
     }
 }
@@ -350,6 +350,8 @@ void problem_update_params(problem *prob, const double *theta)
     {
         expr *pnode = prob->param_nodes[i];
         parameter_expr *param = (parameter_expr *) pnode;
+        if (param->param_id == PARAM_FIXED)
+            continue;
         int offset = param->param_id;
         memcpy(pnode->value, theta + offset, pnode->size * sizeof(double));
         param->has_been_refreshed = false;

tests/all_tests.c

@@ -303,6 +303,7 @@ int main(void)
     mu_run_test(test_param_vector_mult_problem, tests_run);
     mu_run_test(test_param_left_matmul_problem, tests_run);
     mu_run_test(test_param_right_matmul_problem, tests_run);
+    mu_run_test(test_param_fixed_skip_in_update, tests_run);
 #endif /* PROFILE_ONLY */
 
 #ifdef PROFILE_ONLY

tests/problem/test_param_prob.h

@@ -345,4 +345,86 @@ const char *test_param_right_matmul_problem(void)
     return 0;
 }
 
+/*
+ * Test 5: PARAM_FIXED params are skipped by problem_update_params
+ *
+ * Problem: minimize a * sum(log(x)) + b * sum(x), no constraints, x size 2
+ *   a is a FIXED scalar parameter (param_id=PARAM_FIXED, value=2.0)
+ *   b is an updatable scalar parameter (param_id=0)
+ *
+ * At x=[1,2], a=2, b=3:
+ *   obj = 2*(log(1)+log(2)) + 3*(1+2) = 2*log(2) + 9
+ *   gradient = [2/1 + 3, 2/2 + 3] = [5.0, 4.0]
+ *
+ * After update theta={5.0} (only b changes to 5, a stays 2):
+ *   obj = 2*log(2) + 5*3 = 2*log(2) + 15
+ *   gradient = [2/1 + 5, 2/2 + 5] = [7.0, 6.0]
+ */
+const char *test_param_fixed_skip_in_update(void)
+{
+    int n_vars = 2;
+
+    /* Build tree: a * sum(log(x)) + b * sum(x) */
+    expr *x1 = new_variable(2, 1, 0, n_vars);
+    expr *log_x = new_log(x1);
+    double a_val = 2.0;
+    expr *a_param = new_parameter(1, 1, PARAM_FIXED, n_vars, &a_val);
+    expr *a_log = new_scalar_mult(a_param, log_x);
+    expr *sum_a_log = new_sum(a_log, -1);
+
+    expr *x2 = new_variable(2, 1, 0, n_vars);
+    expr *b_param = new_parameter(1, 1, 0, n_vars, NULL);
+    expr *b_x = new_scalar_mult(b_param, x2);
+    expr *sum_b_x = new_sum(b_x, -1);
+
+    expr *objective = new_add(sum_a_log, sum_b_x);
+
+    /* Create problem and register BOTH params */
+    problem *prob = new_problem(objective, NULL, 0, false);
+
+    expr *param_nodes[2] = {a_param, b_param};
+    problem_register_params(prob, param_nodes, 2);
+    problem_init_derivatives(prob);
+
+    /* Set b=3 and evaluate at x=[1,2] */
+    double theta[1] = {3.0};
+    problem_update_params(prob, theta);
+
+    /* Verify a is still 2.0 (not overwritten) */
+    mu_assert("a_param changed after update",
+              fabs(a_param->value[0] - 2.0) < 1e-10);
+
+    double u[2] = {1.0, 2.0};
+    double obj_val = problem_objective_forward(prob, u);
+    problem_gradient(prob);
+
+    double expected_obj = 2.0 * log(2.0) + 9.0;
+    mu_assert("obj wrong (b=3)", fabs(obj_val - expected_obj) < 1e-10);
+
+    double expected_grad[2] = {5.0, 4.0};
+    mu_assert("gradient wrong (b=3)",
+              cmp_double_array(prob->gradient_values, expected_grad, 2));
+
+    /* Update b=5, a should stay 2 */
+    theta[0] = 5.0;
+    problem_update_params(prob, theta);
+
+    mu_assert("a_param changed after second update",
+              fabs(a_param->value[0] - 2.0) < 1e-10);
+
+    obj_val = problem_objective_forward(prob, u);
+    problem_gradient(prob);
+
+    double expected_obj2 = 2.0 * log(2.0) + 15.0;
+    mu_assert("obj wrong (b=5)", fabs(obj_val - expected_obj2) < 1e-10);
+
+    double expected_grad2[2] = {7.0, 6.0};
+    mu_assert("gradient wrong (b=5)",
+              cmp_double_array(prob->gradient_values, expected_grad2, 2));
+
+    free_problem(prob);
+
+    return 0;
+}
+
 #endif /* TEST_PARAM_PROB_H */