Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add counts to problem printing #643

Closed
wants to merge 10 commits into from
Closed

Conversation

ericphanson
Copy link
Collaborator

@ericphanson ericphanson commented May 8, 2024

closes #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: 179837)
subject to
└─ == constraint (affine)
   └─ + (affine; complex)
      ├─ 3×4 complex variable (id: 179837)
      └─ 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: 863744)
│     └─ Convex.NegateAtom (constant; positive)
│        └─ 
└─  constraint (affine)
   └─ + (affine; real)
      ├─ 2730-element real variable (id: 863744)
      └─ 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
Copy link
Collaborator Author

This now also addresses #644:

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

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: 483692)
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
Copy link
Collaborator Author

ericphanson commented May 9, 2024

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.

Copy link

codecov bot commented May 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

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

Additional details and impacted files
@@            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              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ericphanson ericphanson requested a review from odow May 9, 2024 22:41
@ericphanson
Copy link
Collaborator Author

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
Copy link
Member

odow commented May 9, 2024

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
Copy link
Collaborator Author

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

@odow
Copy link
Member

odow commented May 10, 2024

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
Copy link
Collaborator Author

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
Copy link
Member

odow commented May 10, 2024

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
Copy link
Member

odow commented May 10, 2024

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: 136543

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: 136543)
subject to
└─ == constraint (affine)
   └─ + (affine; complex)
      ├─ 3×4 complex variable (id: 136543)
      └─ 3×4 complex constant

status: `solve!` not called yet

and

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

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: 153767)
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
Copy link
Member

odow commented May 13, 2024

Closing in favor of #650

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants