Cache conic forms between solve!s?

[ericphanson]

Right now, calling solve! calls conic_problem which makes a new UniqueConicForms() object, then fills it up by recursing down the tree which constitutes the problem via conic_form!. Then if you say fix! a variable to a different parameter, or change the objective, and solve! again, it again calls UniqueConicForms() to make a unique conic forms object, fills it up, etc.

So if I'm understanding the code correctly, in the end, nothing was saved as compared to making a fresh problem with the new value of the variable; the only caching is between conic forms reused within the same problem present simultaneously when solve! is called.

I think one solution to this is to keep a problem-local unique conic forms object, which holds onto every conic form seen by that problem (not a global one to prevent memory issues like in #83). That way, reusing a problem can reuse the conic forms it's already calculated.

One problem I forsee is fix!ing a variable (or free!ing a fixed one, or changing the value of a fixed variable); we need to recalculate conic forms involving the variable in that case. I think a solution to that when we hash a variable, we include its fix! vs free! status (i.e. the vexity) of the variable, and either its value, or maybe just a counter indicating how many times the value has been changed.

Also, I notice sometimes hash is used just with the children of an atom, and sometimes the head of the atom is included too. I should figure out why that is, and if it could lead to bugs.

The other difficulty is that currently, the whole constr_list of the unique conic forms object is used in conic_problem; if we keep a problem-local running cache, we have to figure out which cached conic forms we actually want to use for the problem.

Another possibly good change is to have UniqueConicForms to hold its own id_to_variables, keeping track of the variables used in the conic forms it caches. That could replace the global id_to_variables dictionary by a local one. The only catch there is that variables constructed internally in the conic_form of expressions need to be included, not just variables directly used in atoms. This can be separate from the problem-local UniqueConicForms though.

[ericphanson]

The OP here is somewhat out of date, but things are mostly the same just renamed (notably UniqueConicForms is now Context).

The impact of this issue is that re-solve!ing the same problem, after e.g. fixing or freeing some variable, or changing what value it is fix!'d to, essentially needs to reformulate the problem from scratch, both within Convex, and also within MOI and the solver. We create a fresh Context each solve, and totally recreate it.

I think it is hard to fix this, because if a Variable has been fix!'d or free!'d, everything in the tree that depends on that variable needs to be invalidated and recreated (since a fix!'d variable is treated just like a Constant, which affects DCPness, conic forms, etc). Moreover, even if we did the bookkeeping to do that and retain the rest of our conic_forms cache, I'm not sure it's tractable to effectively communicate the updates to MOI so the problem doesn't have to be recreated entirely on that side.

I think therefore the only efficient version of "update a Convex.jl model and resolve" would be a parameter which is always fixed, but one can change what value it is fixed to. This could be communicated to MOI via https://github.com/jump-dev/ParametricOptInterface.jl. I think this would still be hard to represent effectively inside Convex.jl itself; I think probably we would want the internal datastructures to be more "symbolic" so we could substitute in the parameter values (just like we treat the actual variables as symbolic).

IMO pretty much any improvement here would be a huge amount of work, so maybe this should just be closed as out-of-scope for now.

(And we could also remove the commented out references to using a WeakIdKeyDict for conic_form_cache inside Context, because that is only useful if the cache is holding things for longer than a single solve, e.g., if we were trying to solve this issue. As-is, I think it's fine if a Context holds references to all the objects in the problem, since they should be live for the duration of the problem until the next solve).

[odow]

I'm okay to close as out-of-scope.

I definitely see the potential for a Convex.jl 2.0 rewrite that fixes how we handle arrays etc, includes parameters, and doesn't rebuild at each step. But I agree that this is a gigantic effort. The focus should be on getting Convex.jl 1.0 released. What we have currently is good and useful.

[odow]

Closing as won't fix for now.

If anyone stumbles across this issue in future and has a compelling use-case, please comment and we can re-consider.