[WIP] Unify constants and parameters into single parameter_expr type#53
[WIP] Unify constants and parameters into single parameter_expr type#53
Conversation
Constants and updatable parameters are now represented by a single parameter_expr node. Constants use param_id == PARAM_FIXED (-1), while updatable parameters use param_id >= 0. Bivariate ops (left_matmul, right_matmul, scalar_mult, vector_mult) accept an optional param_source node and refresh their internal data on forward/jacobian/hessian evaluation. Adds problem_register_params and problem_update_params for problem-level parameter management. Also adds update_values to the Matrix interface for parameter refresh. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- 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>
Resolves conflicts from main's workspace refactor (iwork/dwork -> work->), elementwise atom split (elementwise_univariate -> full_dom/restricted_dom), chain rule refactor, vstack addition, and numerical diff checker. Removed stale test_jacobian_composite_log tests (incompatible with main's chain rule refactor; covered by test_chain_rule_jacobian and numerical diff). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@dance858 I synced the PR with master, then addressed your comments. Please have another look. |
Thanks for looking into the comments. Let me know when you have added tests on the python side and we'll merge this nice PR. Also, sorry for the refactoring which forces you to sync this PR... But in the long run the refactoring is good. |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Resolves conflicts from folder structure refactor (#63) and more tests (#62). Moves scalar_mult/vector_mult to src/affine/ and test files to new subdirectory structure. Updates affine.h declarations with parameter support signatures. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Resolve merge conflicts in parameter.c and scalar_mult.c: keep parameter-support-v2 features (parameter_expr, param_id, free_type_data) while adopting _impl suffix naming from main. Fix old 2-arg new_left_matmul calls in test_chain_rule_jacobian.h to use new 3-arg signature with NULL param_node. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Resolve conflicts from matmul chain rule PR (#67): - subexpr.h: keep both vector_mult_expr and new matmul_expr/const types - dense_matrix.c: adopt shared I_kron_A functions, keep update_values - Add missing dense_matrix.h include in right_matmul.c - Update new matmul tests to use 3-arg left_matmul signature Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
# Conflicts: # src/affine/const_scalar_mult.c # src/affine/const_vector_mult.c # src/affine/constant.c # src/atoms/affine/const_scalar_mult.c # src/atoms/affine/const_vector_mult.c # src/atoms/affine/constant.c # src/atoms/affine/parameter.c # src/atoms/affine/right_matmul.c # src/atoms/affine/scalar_mult.c # src/atoms/affine/vector_mult.c # tests/jacobian_tests/affine/test_scalar_mult.h # tests/jacobian_tests/affine/test_vector_mult.h # tests/wsum_hess/affine/test_scalar_mult.h # tests/wsum_hess/affine/test_vector_mult.h
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Resolve conflicts from tracked allocation PR (#70): use SP_CALLOC/SP_MALLOC macros in parameter-support files while keeping parameter-specific types. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Fix column-major parameter ordering in parameterized matmul CVXPY sends parameter values in Fortran (column-major) order, but the matmul refresh functions assumed row-major/CSR order via raw memcpy. This produced incorrect matrix values for non-symmetric matrices. For sparse matrices, iterate the CSR pattern and index into the column-major source array. For dense matrices, exploit the fact that column-major A is row-major A^T to memcpy directly into AT, then transpose to get A. Also fixes a latent bug where sparse update_values would blindly copy the first nnz values from the full d1*d2 parameter array, which is wrong for matrices with structural zeros. Adds tests for rectangular (3x2) and sparse (3x3 with zeros) cases. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Run clang-format on changed files Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * introduce explicit transpose function for dense matrix * clean up refresh dense right * clean up tests... * one more test --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: dance858 <danielcederberg1@gmail.com>
* add initial attempt for fixing parameters and broadcasting * add test for params with broadcast * cleanup test to fit more with other tests style * some progress on supporting backwards compatible constants * add some parameter broadcast tests as well * cleanup left matmul as well * some very minor cleanups * some error checks and numerical diff to tests * we don't always have to run forward of parameter in left matmul * we don't always have to call forward for parameter node in vector mult * comment out forward parameter pass in scalar mult because it is not needed, I think * add test for scalar case to be consistent with vector mult --------- Co-authored-by: dance858 <danielcederberg1@gmail.com>
|
I'm happy with our abstraction and think this is well-tested, both on the Python side and the C side. So I'm merging this. Great collaboration, @Transurgeon! |
Constants and updatable parameters are now represented by a single parameter_expr node. Constants use param_id == PARAM_FIXED (-1), while updatable parameters use param_id >= 0. Bivariate ops (left_matmul, right_matmul, scalar_mult, vector_mult) accept an optional param_source node and refresh their internal data on forward/jacobian/hessian evaluation. Adds problem_register_params and problem_update_params for problem-level parameter management. Also adds update_values to the Matrix interface for parameter refresh.
▎ - Unifies constants and updatable parameters into a single parameter_expr type (param_id == PARAM_FIXED for constants, >= 0 for updatable parameters)
▎ - Adds update_values to the Matrix interface for parameter refresh (sparse and dense)
▎ - Makes left_matmul, right_matmul, scalar_mult, and vector_mult parameter-aware via optional param_source pointer
▎ - Adds problem_register_params / problem_update_params for problem-level parameter management
▎ - Renames constant.c → parameter.c, const_scalar_mult.c → scalar_mult.c, const_vector_mult.c → vector_mult.c
▎ Test plan
▎ - All 196 existing tests pass (API updated to new signatures)
▎ - 4 new parameter problem-level tests: scalar_mult, vector_mult, left_matmul, right_matmul — each verifies correct evaluation after parameter update
▎ - clang-format clean