jump-dev / Convex.jl

A Julia package for disciplined convex programming
https://jump.dev/Convex.jl/stable/
Other
559 stars 119 forks source link

Add counts to problem printing #643

Closed ericphanson closed 2 months ago

ericphanson commented 2 months ago

closes https://github.com/jump-dev/Convex.jl/issues/640 closes #644

I haven't written tests yet since I'd like to get feedback on the printing before that. The problem

A = [1 2im 3 4; 4im 3im 2 1; 4 5 6 7]
y = ComplexVariable(3, 4)
p = minimize(nuclearnorm(y), y == A)

results in

julia> p = minimize(nuclearnorm(y), y == A)
minimize
└─ nuclearnorm (convex; positive)
   └─ 3×4 complex variable (id: 179…837)
subject to
└─ == constraint (affine)
   └─ + (affine; complex)
      ├─ 3×4 complex variable (id: 179…837)
      └─ 3×4 complex constant
contains: 1 variable (24 elements), 1 constraint (24 elements), 1 constant (24 dense elements), and 3 atoms (total: 736 bytes)
status: `solve!` not called yet

where the "contains" line is new.

The problem from #254 in our benchmarks results in

julia> problem
maximize
└─ + (concave; real)
   ├─ Convex.NegateAtom (concave; real)
   │  └─ + (convex; real)
   │     ├─ …
   │     └─ …
   └─ Convex.NegateAtom (affine; real)
      └─ sum (affine; real)
         └─ …
subject to
├─ ≥ constraint (affine)
│  └─ + (affine; real)
│     ├─ 2730-element real variable (id: 863…744)
│     └─ Convex.NegateAtom (constant; positive)
│        └─ …
└─ ≤ constraint (affine)
   └─ + (affine; real)
      ├─ 2730-element real variable (id: 863…744)
      └─ Convex.NegateAtom (constant; negative)
         └─ …
contains: 1 variable (2_730 elements), 2 constraints (5_460 elements), 8 constants (9_462 dense elements, 22_000 sparse nonzeros), and 22 atoms (total: 456.781 KiB)
status: `solve!` not called yet

etc.

Note that the counts are of the problem as formulated by the user, NOT as reformulated by Convex during conic_form. Since show can happen before the MOI model is formulated, and I do the counting on the Conevx.jl expression tree.

I think this is good and bad. The good is that the values are easy to understand; you get 1 variable since you wrote down 1 variable, etc. If the user does a bunch of scalar indexing etc, they will get many atoms. However, it is not so easy to use these counts to notice performance issues stemming from bad conic_form! reformulations; e.g., we would see the same values pre- and post- #618 since the change was only in conic_form!. But we shouldn't have many of those anymore, so maybe it's not a big deal.

We could also count things at the MOI level post-reformulation. I think that is less useful (especially for show), since it's nice to get a quick sense of the problem before you go to solve it.

ericphanson commented 2 months ago

This now also addresses https://github.com/jump-dev/Convex.jl/issues/644:

julia> x = Variable(1000)
Variable
size: (1000, 1)
sign: real
vexity: affine
id: 483…692

julia> for i in 1:1000
       add_constraint!(x, x[i] >= 0)
       end

julia> p = minimize(sum(x))
minimize
└─ sum (affine; real)
   └─ 1000-element real variable (id: 483…692)
subject to
├─ ≥ constraint (affine)
│  └─ + (affine; real)
│     ├─ index (affine; real)
│     │  └─ …
│     └─ [0;;]
├─ ≥ constraint (affine)
│  └─ + (affine; real)
│     ├─ index (affine; real)
│     │  └─ …
│     └─ [0;;]
├─ ≥ constraint (affine)
│  └─ + (affine; real)
│     ├─ index (affine; real)
│     │  └─ …
│     └─ [0;;]
├─ ≥ constraint (affine)
│  └─ + (affine; real)
│     ├─ index (affine; real)
│     │  └─ …
│     └─ [0;;]
├─ ≥ constraint (affine)
│  └─ + (affine; real)
│     ├─ index (affine; real)
│     │  └─ …
│     └─ [0;;]
├─ ≥ constraint (affine)
│  └─ + (affine; real)
│     ├─ index (affine; real)
│     │  └─ …
│     └─ [0;;]
├─ ≥ constraint (affine)
│  └─ + (affine; real)
│     ├─ index (affine; real)
│     │  └─ …
│     └─ [0;;]
├─ ≥ constraint (affine)
│  └─ + (affine; real)
│     ├─ index (affine; real)
│     │  └─ …
│     └─ [0;;]
├─ ≥ constraint (affine)
│  └─ + (affine; real)
│     ├─ index (affine; real)
│     │  └─ …
│     └─ [0;;]
├─ ≥ constraint (affine)
│  └─ + (affine; real)
│     ├─ index (affine; real)
│     │  └─ …
│     └─ [0;;]
├─ ≥ constraint (affine)
│  └─ + (affine; real)
│     ├─ index (affine; real)
│     │  └─ …
│     └─ [0;;]
├─ ≥ constraint (affine)
│  └─ + (affine; real)
│     ├─ index (affine; real)
│     │  └─ …
│     └─ [0;;]
├─ ≥ constraint (affine)
│  └─ + (affine; real)
│     ├─ index (affine; real)
│     │  └─ …
│     └─ [0;;]
├─ ≥ constraint (affine)
│  └─ + (affine; real)
│     ├─ index (affine; real)
│     │  └─ …
│     └─ [0;;]
├─ ≥ constraint (affine)
│  └─ + (affine; real)
│     ├─ index (affine; real)
│     │  └─ …
│     └─ [0;;]
⋮
contains: 1 variable (1_000 elements), 1_000 constraints (1_000 elements), 1_000 constants (1_000 dense elements), and 2_002 atoms (total: 273.656 KiB)
status: `solve!` not called yet
ericphanson commented 2 months ago

I think I'd like to also include counts from MOI whenever the MOI model is populated. Something like

         └─ …
contains: 1 variable (2_730 elements), 2 constraints (5_460 elements), 8 constants (9_462 dense elements, 22_000 sparse nonzeros), and 22 atoms (total: 25.409 MiB)
reformulation: 6_732 scalar variables, 4_004 constraints (17_462 elements), 74_922 sparse constants (total: 24.900 MiB)
termination status: OPTIMAL
primal status: FEASIBLE_POINT
dual status: FEASIBLE_POINT

using


function get_counts(model)
    n_constraints = 0
    n_elements = 0
    n_variables = MOI.get(model, MOI.NumberOfVariables())
    n_constants = 0
    for (F, S) in MOI.get(model, MOI.ListOfConstraintTypesPresent())
        n_constraints += MOI.get(problem.model, MOI.NumberOfConstraints{F,S}())

        for idx in MOI.get(model, MOI.ListOfConstraintIndices{F,S}())
            set = MOI.get(model, MOI.ConstraintSet(), idx)
            n_elements += MOI.dimension(set)
            f = MOI.get(model, MOI.ConstraintFunction(), idx)
            n_constants += count_size(f)
        end
    end
    return (; n_variables, n_constraints, n_elements, n_constants)
end

function count_size(f::MOI.VectorAffineFunction)
    return length(f.terms) + length(f.constants)
end

function count_size(x)
    # TODO
    @show typeof(x)
    @show propertynames(x)
    return 0
end

edit: just realized the summarysize for the problem includes that of the model; I would prefer it was just the expression tree... will need to think about that.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 97.93%. Comparing base (19ab3a9) to head (d9955df).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #643 +/- ## ========================================== + Coverage 97.88% 97.93% +0.04% ========================================== Files 88 89 +1 Lines 5112 5225 +113 ========================================== + Hits 5004 5117 +113 Misses 108 108 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ericphanson commented 2 months ago

just realized the summarysize for the problem includes that of the model; I would prefer it was just the expression tree... will need to think about that.

I just subtract the size of the model. When it is nothing it is 0.

odow commented 2 months ago

I don't know that I like this. It feels somewhat wrong.

I wonder if we should have something like JuMP's distinction between print and show.

The expression graph is what I want when I print(p).

A summary of "this problem has X atoms, and takes Y MB" seems more like a summary that we could display on show.

ericphanson commented 2 months ago

Hm, so would that be to move the existing expression tree printing to print and then use these counts for show?

odow commented 2 months ago

I don't really know what I want. This seems like a lot of overhead. JuMP can efficiently query the model for how many variables and constraints there are. We can't really do that easily here...

ericphanson commented 2 months ago

My thinking was just iterating the tree should be way cheaper than doing all the formulations and sending it to MOI, so it should be somewhat negligible, and “in production” you probably aren’t printing anyway. Maybe I can do some benchmarks here to try to see the cost?

odow commented 2 months ago

It's not the computational cost I'm worried about. I get you don't print often. There's just a lot of cruft counting up things. It adds a bunch of code for low utility.

Perhaps I just need to think through this a bit more. I wonder how else we could structure the prints. Would it be helpful to attach more information to each atom or each constraint?

odow commented 2 months ago

How about something like this:

julia> A = [1 2im 3 4; 4im 3im 2 1; 4 5 6 7]
3×4 Matrix{Complex{Int64}}:
 1+0im  0+2im  3+0im  4+0im
 0+4im  0+3im  2+0im  1+0im
 4+0im  5+0im  6+0im  7+0im

julia> y = ComplexVariable(3, 4)
Variable
size: (3, 4)
sign: complex
vexity: affine
id: 136…543

julia> p = minimize(nuclearnorm(y), y == A)
summary
├─ # variables    : 1 (24 scalar elements)
├─ # constraints  : 1 (24 scalar elements)
├─ # coefficients : 24
├─ # atoms        : 2
└─ size           : 704 bytes

minimize
└─ nuclearnorm (convex; positive)
   └─ 3×4 complex variable (id: 136…543)
subject to
└─ == constraint (affine)
   └─ + (affine; complex)
      ├─ 3×4 complex variable (id: 136…543)
      └─ 3×4 complex constant

status: `solve!` not called yet

and

julia> x = Variable(1000)
Variable
size: (1000, 1)
sign: real
vexity: affine
id: 153…767

julia> for i in 1:1000
       add_constraint!(x, x[i] >= 0)
       end

julia> p = minimize(sum(x))
summary
├─ # variables    : 1 (1_000 scalar elements)
├─ # constraints  : 1_000 (1_000 scalar elements)
├─ # coefficients : 1000
├─ # atoms        : 2001
└─ size           : 265.836 KiB

minimize
└─ sum (affine; real)
   └─ 1000-element real variable (id: 153…767)
subject to
├─ ≥ constraint (affine)

I don't think we should report the MOI counts. We could add something to MOI to improve that, or add a default show method.

I'm also not sure about the utility of printing all of the dense and sparse non-zero statistics.

odow commented 2 months ago

Closing in favor of #650