Improve printing of problems, constraints, and variables

[ericphanson]

I find myself usually ignoring the printing of problems, because it's quite verbose while not being very informative. Here, I propose new printing that I think helps with both problems.

Old printing:

julia> x = Variable(4, 1)
Variable of
size: (4, 1)
sign: NoSign()
vexity: AffineVexity()

julia> p = minimize(dotsort(x, [1, 2, 3, 4]), sum(x) >= 7, x >= 0, x <= 2, x[4] <= 1)
Problem:
minimize AbstractExpr with
head: dotsort
size: (1, 1)
sign: NoSign()
vexity: ConvexVexity()

subject to
Constraint:
>= constraint
lhs: AbstractExpr with
head: sum
size: (1, 1)
sign: NoSign()
vexity: AffineVexity()

rhs: 7
vexity: AffineVexity()
                Constraint:
>= constraint
lhs: Variable of
size: (4, 1)
sign: NoSign()
vexity: AffineVexity()
rhs: 0
vexity: AffineVexity()
                Constraint:
<= constraint
lhs: Variable of
size: (4, 1)
sign: NoSign()
vexity: AffineVexity()
rhs: 2
vexity: AffineVexity()
                Constraint:
<= constraint
lhs: AbstractExpr with
head: index
size: (1, 1)
sign: NoSign()
vexity: AffineVexity()

rhs: 1
vexity: AffineVexity()
current status: not yet solved

After:

julia> x = Variable(4, 1)
Variable
size: (4, 1)
sign: real
vexity: affine
id: 1033...

julia> p = minimize(dotsort(x, [1, 2, 3, 4]), sum(x) >= 7, x >= 0, x <= 2, x[4] <= 1)
minimize
└─ dotsort (convex; real)
   └─ 4-element real variable (id: 1033...)
subject to
├─ >= constraint (affine)
│  ├─ sum (affine; real)
│  │  └─ 4-element real variable (id: 1033...)
│  └─ 7
├─ >= constraint (affine)
│  ├─ 4-element real variable (id: 1033...)
│  └─ 0
├─ <= constraint (affine)
│  ├─ 4-element real variable (id: 1033...)
│  └─ 2
└─ <= constraint (affine)
   ├─ index (affine; real)
   │  └─ 4-element real variable (id: 1033...)
   └─ 1

current status: not yet solved

It's both shorter and much more informative: the old printing does not recurse down the tree. So for example, for the last constraint in the above problem, with the old way of printing you see

<= constraint
lhs: AbstractExpr with
head: index
size: (1, 1)
sign: NoSign()
vexity: AffineVexity()

rhs: 1
vexity: AffineVexity()

which doesn't tell you what is being indexed. Now you see

└─ <= constraint (affine)
   ├─ index (affine; real)
   │  └─ 4-element real variable (id: 1033...)
   └─ 1

which tells you what variable is being indexed. This is only compounded for large problems. Note, we only recurse three levels down by default, so recursion shouldn't cause problems if a huge problem gets printed. This can be controlled by Convex.MAXDEPTH[] = 5 e.g., for users who wish to customize this.

I think the new printing will often be shorter than the current printing even accounting for recursion: adding a constraint in the current method of printing means 10 more lines get printed when the problem is printed. With the recursion depth set to three, it is possible to add a constraint yielding more lines, but the worst case is 15 lines (nested binary operators all the way down on both sides):

julia> x = Variable(2);

julia> y = Variable(2);

julia> p = minimize(sum(x), hcat(hcat(hcat(hcat(x,y), hcat(x,y)),hcat(hcat(x,y), hcat(x,y))),hcat(hcat(hcat(x,y), hcat(x,y)),hcat(hcat(x,y), hcat(x,y)))) == hcat(hcat(hcat(hcat(x,y), hcat(x,y)),hcat(hcat(x,y), hcat(x,y))),hcat(hcat(hcat(x,y), hcat(x,y)),hcat(hcat(x,y), hcat(x,y)))))
minimize
└─ sum (affine; real)
   └─ 2-element real variable (id: 1024...)
subject to
└─ == constraint (affine)
   ├─ hcat (affine; real)
   │  ├─ hcat (affine; real)
   │  │  ├─ …
   │  │  └─ …
   │  └─ hcat (affine; real)
   │     ├─ …
   │     └─ …
   └─ hcat (affine; real)
      ├─ hcat (affine; real)
      │  ├─ …
      │  └─ …
      └─ hcat (affine; real)
         ├─ …
         └─ …

current status: not yet solved

and most times it will be less (unary operators, or reaching a leaf such as a variable or constant).

Why am I trying to justify the improvements so much? Because it doesn't come for free: I'm using Keno's AbstractTrees.jl to do the hard work of printing these nice trees, so we would need to take on a dependency on that package. It's 700 lines of Julia with no dependencies, so it's quite a light dependency. It also means Convex problems can be treated as trees in other packages that respect the AbstractTrees API. For example, using GraphRecipes master and this branch, one can do

using Convex, GraphRecipes, Plots
plot(TreePlot(p); nodeshape=:rect, method=:tree)

where p::Problem is a Convex.jl problem. It doesn't look perfect though:

(overlapping nodes etc).
Still, it's kind of cool that it's easily integrated, and maybe stuff like that can be improved on the GraphRecipes side. Another example of what you can do is one I added to the docs: use AbstractTrees to iterate over the problem in various orders, or iterate over the "leaves" of the problem, and so forth. For example

using Convex, AbstractTrees
x = Variable()
p = maximize( log(x), x >= 1, x <= 3 )
for leaf in AbstractTrees.Leaves(p)
    println("Here's a leaf: $(summary(leaf))")
end

yields

Here's a leaf: real variable (id: 3942...)
Here's a leaf: real variable (id: 3942...)
Here's a leaf: constant (constant; positive)
Here's a leaf: real variable (id: 3942...)
Here's a leaf: constant (constant; positive)

One can also iterate over the problem in various orders, as shown in the docs in this PR. I imagine these iterators could be useful for figuring out what's going on in a problem. For example, one could count how many of a certain kind of atom is in the problem, etc, particularly if parts of the problem are being programatically generated, which might be the case for DSLs sitting on top of Convex. Note, however, we don't recurse into the internal structure of an atom, and some atoms create internal variables and constraints etc in the conic form; this doesn't help for seeing into that.

This is optional item 1 of the list in #320. I initially wasn't sold on adding a dependency to Convex, nor complicating the printing of its objects, but after using this, it's hard to go back.

Note: AbstractTrees.jl does not actually respect the maxdepth argument for print_tree (it looks like it's just not implemented). I made a PR to fix that (JuliaCollections/AbstractTrees.jl#38), but it might be some time before that gets merged, judging by the repo's activity. So I vendored tree_print with the changes from that PR to a submodule in utilities/tree_print.jl. When the PR to AbstractTrees (or equivalent) eventually goes through, we can just replace all TreePrint.print_tree by AbstractTrees.print_tree, and delete the file utilities/tree_print.jl.

Fun note: somehow I've been very into improving printing the past few days-- a kind of sibling PR to this is pretty printing for MOI optimizers: jump-dev/MathOptInterface.jl#864.

[ericphanson]

I'll merge this in 48 hours if I don't hear any objections before then. These changes are orthogonal to eg MOI, global variables, etc, so this should be easily revertible in case it turns out there are strong objections to the AbstractTrees dependency.

[ericphanson]

@matbesancon I'd be happy to get a review if you have a chance :). If you want to review but don't have time in the next few days, I don't mind holding off merging, either; the 48 hours I mention above was just an arbitrary 1-week wait that I thought was a suitable time to wait for comments in case some were coming.

[matbesancon]

Sorry about the delay on this, and thanks for that work.

1. I'm ok with the dependency on abstract trees, it looks reasonable and generally helpful
2. New printing looks good, too verbose a default is always discouraging.
3. When giving the id, why id 123... and not id 123?

[matbesancon]

I don't think this is needed, AbstractTrees is a dependency, you did not copy code directly right?

[ericphanson]

Yes, I did copy code directly in the TreePrint module, just to use JuliaCollections/AbstractTrees.jl#38 on our side. We can delete the module and the copied code once that PR is merged, but I don't think it will be soon since the repo seems pretty inactive.

[ericphanson]

Sorry about the delay on this, and thanks for that work.

1. I'm ok with the dependency on abstract trees, it looks reasonable and generally helpful
2. New printing looks good, too verbose a default is always discouraging.
3. When giving the id, why id 123... and not id 123?

Thanks! Regarding the id, good question. The id is just the first few characters of the id_hash field on the Variable (which is really long). With 4 characters, there's a decent chance of collision still (i.e. two variables have their id_field starting with the same first four characters). I thought that could be very confusing, so I put the ... to indicate this was just part of the id.

I could make these dots take a bit less space by using the same ellipsis character from what gets printed when the tree exceeds its maxdepth (…).

What do you think?

[matbesancon]

Ok thanks for the precision, if it's not too complex 123...465 would be perfect to understand that the id has been truncated, but it might be me being slow to understand 😬

[ericphanson]

Ok thanks for the precision, if it's not too complex 123...465 would be perfect to understand that the id has been truncated, but it might be me being slow to understand 😬

That's a great idea, I think that'll make it really clear. I'll push a commit with that change in a minute.