Adds convert function from cvxpy problem to C#3
Conversation
There was a problem hiding this comment.
I added this file because this is my plan for the next step (I used claude code's plan mode).
I think it makes sense to ultimately create an equivalent problem in C (which will have objective and list of constraints).. that way we can have a really nice abstraction from the python side (it will just need to use the problem struct's oracles methods).
Let me know what you think about this.
There was a problem hiding this comment.
I think this will also be nice for computing the jacobian of constraints. We can apply the row offsets I mentioned earlier. Currently there is no simple way to test the jacobian of constraints (we would need to loop through all constraints everytime).
There was a problem hiding this comment.
I added this file because this is my plan for the next step (I used claude code's plan mode). I think it makes sense to ultimately create an equivalent problem in C (which will have objective and list of constraints).. that way we can have a really nice abstraction from the python side (it will just need to use the problem struct's oracles methods). Let me know what you think about this.
I really like this abstraction and the idea of a problem struct in C.
The problem struct must allocate memory for the constraint values, for the jacobian, and for the hessian of the Lagrangian. You can have a function that does this (I would put this functionality outside the new_problem constructor):
- allocate the array for the constraint values that must be of size num_of_constraints
- to allocate the jacobian, first initialize all your constraint functions jacobians. Then loop through them to count the nnz. Allocate a CSR matrix in prob_struct with this number of nnz. (This will give a slightly overestimate on the nnz, but it should be fine. Just remember to actually update the nnz of the jacobian field in the problem struct once we evaluate the jacobian.)
- let's wait with allocating space for the hessian
| static int numpy_initialized = 0; | ||
|
|
||
| static int ensure_numpy(void) | ||
| { | ||
| import_array(); | ||
| if (numpy_initialized) return 0; | ||
| import_array1(-1); | ||
| numpy_initialized = 1; |
There was a problem hiding this comment.
I am not sure why this change was made. I could try to see if it works without it if necessary.
But something was wrong with just import_array so there was definitely a reason to do this.
| return out; | ||
| } | ||
|
|
||
| static PyObject *py_jacobian(PyObject *self, PyObject *args) |
There was a problem hiding this comment.
can you double check that this implementation makes sense? It seems to work atleast for a single expression.
There was a problem hiding this comment.
Is it correct that the purpose of this function right now is just to construct easy Python tests, say?
If yes, the implementation looks good.
We won't need this later if we use the idea of the problem struct in C, right?
If we actually want to use this function in production we should separate the forward, initialization, and eval_jacobian functions. But I don't think we will use it in production?
python/convert.py
Outdated
| def test_log_exp_identity(): | ||
| """Test sum(log(exp(x))) = sum(x) identity - nested elementwise.""" | ||
| x = cp.Variable(5) | ||
| problem = cp.Problem(cp.Minimize(cp.sum(cp.log(cp.exp(x))))) |
There was a problem hiding this comment.
this won't be allowed in our canonicalization. But just a sanity check for now.
In general, I will cleanup those tests, they are just here to show functionality for now.
|
This looks great! I commented on your idea of a problem struct above and gave some hints for an implementation. Can we move the Python tests to a separate folder inside the python folder? You can have one python file for now with all the tests, but eventually we might want to structure them a bit. Once you have moved the python tests to a separate folder, feel free to merge this PR. Huge progress man. Very nice job! |
2 participants
This PR adds functionality to convert expression trees of a cvxpy problem into their C equivalents.
One difficulty that immediately came to me, is how to provide the field var_id (we will rename it later to offset).
I thought of a clean solution, which also reuses C variables, to get this.
The semantics are as follows:
InverseData.InverseData, we can build a variable dictionary which maps the cvxpy variable ID to its equivalent C variable, we also retrieve the correct variable offset in the meantime._convert_exprwhich uses a ATOM_CONVERTERS dictionary to dynamically create a C expression corresponding to the current atom.The tests show that this works for a variety of cvxpy problems (only forward tests, and a few jacobian ones for now).
@dance858 please double check that the changes in the bindings make sense. I must warn you that this PR was generated with claude code :) .