[Ready for review] Adds problem struct, neg and promote atoms and a lot more tests by Transurgeon · Pull Request #5 · SparseDifferentiation/SparseDiffEngine

Transurgeon · 2026-01-05T05:20:03Z

This PR adds the problem struct. It is currently still in draft mode because I have yet to give a thorough review of what Claude has vibed out. But I think it is mostly in a good state, so I thought you could have a look and give some initial feedback.

I have added your feedback and made sure the problem struct would use pointers for the jacobian, gradient and constraint values. The constraints are stored as a pointer to a pointer.. I think it makes sense, since we want to know where the list begins in memory.

I will also try to cleanup the tests and review them more carefully.

The next step will be to shortcut the expression trees and form the A matrices of affine arguments using the CoeffExtractor. I had initial doubts that it would work.. but after more thought being put into it I think it will work just fine.
My concern was as follows: we might have to build the corresponding x in \phi(Ax) by retrieving the variables that appear in the tree and zeroing out everything else. However, this is not true, since the A in question is global, the columns of variables that don't appear will already be filled with zeros.. so we can actually replace the entire affine subtree with this A in the convert method.

src/problem.c

dance858 · 2026-01-05T11:51:24Z

I did a quick scan and it looks good. But I want to go through it very carefully once you think it's ready.

Add some more tests, and carefully check them. For example, in test_problem.h there seems to be no test for a problem with several constraints.

Nice job!

python/bindings.c

src/problem.c

include/problem.h

src/affine/neg.c

Transurgeon

I think this is good to go.. many more changes since we last talked, but I have a much better understanding of the tests (I did look at them, finally) and the overall code structure.
Here's the structure:

test_native.py just tests manual formation of a problem using the python bindings. By manual, I mean trees are formed manually.
test_unconstrained.py tests the objective and gradient of many different problems.
test_constrained.py tests the constraint forward and jacobian for many other different problems.
I also added a few C tests (similar to test_native) and some Claude generated tests for neg and promote (please double check those and the implementations carefully.. they seem fine at first glance, but maybe some subtle errors can be there).

Let's go man! Huge progress today. Next step will be to add the retrieval of LinearOperators and getting them to work.

python/bindings.c

src/problem.c

dance858 · 2026-01-06T10:48:13Z

Huge progress man! You're a machine.

I left a few comments.

Perhaps it's good if you don't add any code with new functionality to this PR. It would be nice to get it merged soon!

Split Python binding functions into separate header files: - atoms/: one file per atom type (variable, constant, linear, log, exp, add, sum, neg, promote) - problem/: one file per method (make_problem, init_derivatives, objective_forward, constraint_forward, gradient, jacobian) Removed standalone forward/jacobian bindings (use problem_* instead). bindings.c now only contains includes and module definition. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Values are now accessed directly from the problem struct: - prob->constraint_values for constraint forward results - prob->gradient_values for objective gradient - prob->stacked_jac for constraint jacobian Also updated Python bindings and tests to use the new API. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

src/affine/promote.c

src/problem.c

dance858 · 2026-01-07T07:27:51Z

Nice, we're getting there. I left a few easy to fix comments

- Replace manual loops with memcpy for copying row pointers and column indices in neg.c and promote.c - Move workspace allocation from eval_wsum_hess to wsum_hess_init in promote.c, using memset to zero before use - Remove unused promote_expr struct (had no extra fields beyond base) - Simplify new_promote to use new_expr directly 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

- promote.c: Use memcpy for wsum_hess values in eval_wsum_hess - problem.c: Simplify row pointer loop to directly copy cjac->p with offset 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Transurgeon · 2026-01-07T21:02:20Z

@dance858 just committed fixes to your review, thanks again!
I am literally so useless without Claude haha.. but it is what it is, its a great tool and we are getting stuff done :).

dance858 · 2026-01-08T07:45:51Z

@dance858 just committed fixes to your review, thanks again! I am literally so useless without Claude haha.. but it is what it is, its a great tool and we are getting stuff done :).

You are a machine man! A well-crafted machine from Canada. Keep the good work up.

tests/forward_pass/affine/test_promote.h

This matches CVXPY's promote atom behavior which only broadcasts scalars to vectors/matrices. Removes vector-to-vector promotion support and the associated dwork allocation. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

src/affine/promote.c

tests/forward_pass/affine/test_promote.h

- Use memcpy instead of loops for copying jacobian indices and values - Use pointer arithmetic style (ptr + offset) instead of &ptr[offset] - Remove unnecessary nnz == 0 guard in jacobian_init - Move jacobian tests from forward_pass/ to jacobian_tests/ Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

tests/problem/test_problem.h

tests/forward_pass/affine/test_neg.h

dance858 · 2026-01-09T05:52:50Z

Awesome, I'm approving this PR now. Once you have fixed the small comments above, please merge it.

Very nice job WZZ!!!

Remove duplicate function definition that was causing compilation errors, likely introduced during merge from main. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Replace manual element-by-element array assertions with helper functions for cleaner, more maintainable test code. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Relocate test_neg_jacobian and test_neg_chain from forward_pass/affine/ to jacobian_tests/ for better test organization. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Transurgeon added 4 commits January 5, 2026 00:03

adds problem struct

7a09d56

adds newly added files

acdd79e

fix freeing of expressions in problem

d1ae7d5

free expressions recursively in problem

e12c752

dance858 reviewed Jan 5, 2026

View reviewed changes

src/problem.c Outdated Show resolved Hide resolved

dance858 reviewed Jan 5, 2026

View reviewed changes

src/problem.c Outdated Show resolved Hide resolved

adds shared variables and freeing them using a visited node tracker

cca4fbc

Transurgeon commented Jan 5, 2026

View reviewed changes

python/bindings.c Outdated Show resolved Hide resolved

Transurgeon commented Jan 5, 2026

View reviewed changes

src/problem.c Outdated Show resolved Hide resolved

Transurgeon commented Jan 5, 2026

View reviewed changes

src/problem.c Outdated Show resolved Hide resolved

Transurgeon commented Jan 5, 2026

View reviewed changes

include/problem.h Outdated Show resolved Hide resolved

Transurgeon added 3 commits January 5, 2026 15:26

adds constraint forward as well

8ea75ab

begin iterating on conversion of real problem, requires neg and promote

4c94fb7

adds new files

9a8cb01

Transurgeon commented Jan 6, 2026

View reviewed changes

src/affine/neg.c Show resolved Hide resolved

cleanup a bit convert.py

279c582

Transurgeon changed the title ~~Draft: Adds problem struct and more tests~~ [Ready for review] Adds problem struct, neg and promote atoms and a lot more tests Jan 6, 2026

Transurgeon commented Jan 6, 2026

View reviewed changes

dance858 reviewed Jan 6, 2026

View reviewed changes

python/bindings.c Outdated Show resolved Hide resolved

dance858 reviewed Jan 6, 2026

View reviewed changes

src/problem.c Outdated Show resolved Hide resolved

dance858 reviewed Jan 6, 2026

View reviewed changes

src/problem.c Outdated Show resolved Hide resolved

dance858 reviewed Jan 6, 2026

View reviewed changes

src/problem.c Outdated Show resolved Hide resolved

dance858 reviewed Jan 6, 2026

View reviewed changes

src/problem.c Outdated Show resolved Hide resolved

Transurgeon and others added 5 commits January 6, 2026 12:40

remove dead code in problem.c

2721ae8

Merge branch 'main' into problem-struct

e2bca37

change problem_allocate to init_deriv

4f0fb21

dance858 reviewed Jan 7, 2026

View reviewed changes

src/affine/promote.c Outdated Show resolved Hide resolved

dance858 reviewed Jan 7, 2026

View reviewed changes

src/affine/promote.c Outdated Show resolved Hide resolved

dance858 reviewed Jan 7, 2026

View reviewed changes

src/affine/promote.c Show resolved Hide resolved

dance858 reviewed Jan 7, 2026

View reviewed changes

src/affine/promote.c Outdated Show resolved Hide resolved

dance858 reviewed Jan 7, 2026

View reviewed changes

src/problem.c Outdated Show resolved Hide resolved

Transurgeon and others added 2 commits January 7, 2026 15:48

dance858 reviewed Jan 8, 2026

View reviewed changes

tests/forward_pass/affine/test_promote.h Outdated Show resolved Hide resolved

Simplify promote to only handle scalar inputs

856ca50

This matches CVXPY's promote atom behavior which only broadcasts scalars to vectors/matrices. Removes vector-to-vector promotion support and the associated dwork allocation. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

dance858 reviewed Jan 8, 2026

View reviewed changes

src/affine/promote.c Outdated Show resolved Hide resolved

dance858 reviewed Jan 8, 2026

View reviewed changes

src/affine/promote.c Outdated Show resolved Hide resolved

dance858 reviewed Jan 8, 2026

View reviewed changes

src/affine/promote.c Show resolved Hide resolved

dance858 reviewed Jan 8, 2026

View reviewed changes

tests/forward_pass/affine/test_promote.h Outdated Show resolved Hide resolved

dance858 reviewed Jan 8, 2026

View reviewed changes

tests/forward_pass/affine/test_promote.h Outdated Show resolved Hide resolved

Transurgeon and others added 2 commits January 8, 2026 14:16

Merge branch 'main' into problem-struct

40a8262

dance858 reviewed Jan 9, 2026

View reviewed changes

tests/problem/test_problem.h Outdated Show resolved Hide resolved

dance858 reviewed Jan 9, 2026

View reviewed changes

tests/forward_pass/affine/test_neg.h Outdated Show resolved Hide resolved

dance858 reviewed Jan 9, 2026

View reviewed changes

tests/forward_pass/affine/test_neg.h Outdated Show resolved Hide resolved

dance858 approved these changes Jan 9, 2026

View reviewed changes

Transurgeon and others added 4 commits January 9, 2026 12:07

Merge branch 'main' into problem-struct

472a4fb

Fix duplicate eval_jacobian definition in variable.c

40927d5

Remove duplicate function definition that was causing compilation errors, likely introduced during merge from main. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Refactor problem tests to use cmp_double_array and cmp_int_array

78fd177

Replace manual element-by-element array assertions with helper functions for cleaner, more maintainable test code. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Move neg jacobian tests to tests/jacobian_tests/test_neg.h

55ca55f

Relocate test_neg_jacobian and test_neg_chain from forward_pass/affine/ to jacobian_tests/ for better test organization. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Transurgeon merged commit 56a5bc9 into main Jan 9, 2026
9 checks passed

dance858 deleted the problem-struct branch January 11, 2026 10:37

Conversation

Transurgeon commented Jan 5, 2026

Uh oh!

Uh oh!

Uh oh!

dance858 commented Jan 5, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Transurgeon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dance858 commented Jan 6, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dance858 commented Jan 7, 2026

Uh oh!

Transurgeon commented Jan 7, 2026

Uh oh!

dance858 commented Jan 8, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dance858 commented Jan 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants