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 basic dualization comparison to simple examples #3408

Closed
wants to merge 13 commits into from

Conversation

jd-foster
Copy link
Collaborator

@jd-foster jd-foster commented Jun 6, 2023

Happy for this be a separate PR: produces this table at the end of simple_examples:

Model Name         #PrimalVars #Cons #DualVars #DualCons
------------------------------------------------------------
theta_problem           15      7       6       1
minimum_distortion      11      15      14      15
max_cut                 10      5       4       1
k_means_clustering      21      29      28      22

https://jump.dev/JuMP.jl/previews/PR3408/tutorials/conic/simple_examples/

@codecov
Copy link

codecov bot commented Jun 6, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.02 🎉

Comparison is base (22719fa) 98.05% compared to head (fda087b) 98.07%.

Additional details and impacted files
@@                Coverage Diff                 @@
##           od/dualization    #3408      +/-   ##
==================================================
+ Coverage           98.05%   98.07%   +0.02%     
==================================================
  Files                  34       34              
  Lines                4926     4928       +2     
==================================================
+ Hits                 4830     4833       +3     
+ Misses                 96       95       -1     

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

println("------------------------------------------------------------")
for (sym, model) in sdp_model_dict
## Skip models with constraints types not yet supported:
if sym in [:correlation_problem, :robust_uncertainty_sets]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's not supported exactly ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For correlation_problem:

ERROR: Constraints of the Function MathOptInterface.VectorAffineFunction{Float64} in the set MathOptInterface.PositiveSemidefiniteConeSquare are not yet implemented.
Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:33
 [2] supported_constraints(con_types::Vector{Tuple{Type, Type}})
   @ Dualization ~/.julia/packages/Dualization/Xb4KB/src/supported.jl:15

For robust_uncertainty_sets:

ERROR: Constraints of the Function MathOptInterface.ScalarAffineFunction{Float64} in the set MathOptInterface.Interval{Float64} are not yet implemented.
Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:33
 [2] supported_constraints(con_types::Vector{Tuple{Type, Type}})
   @ Dualization ~/.julia/packages/Dualization/Xb4KB/src/supported.jl:15

@odow odow force-pushed the od/dualization branch from ca6a6b5 to fdc4425 Compare June 6, 2023 23:06
Base automatically changed from od/dualization to master June 7, 2023 22:25
@odow
Copy link
Member

odow commented Jun 7, 2023

I'm not sure if we need this part. Maybe let's hold off and see if we get any comments on discourse.

@jd-foster
Copy link
Collaborator Author

That's fine, probably need to resolve the dualization of the two outstanding problems anyway before adding this.

@odow
Copy link
Member

odow commented Jun 7, 2023

PSD cone issue is jump-dev/Dualization.jl#73. I don't know if Interval has a nice dual set.

@odow odow closed this Jun 8, 2023
@odow odow deleted the jdf/dualization-simple branch June 8, 2023 00:00
@blegat blegat restored the jdf/dualization-simple branch June 8, 2023 10:58
@blegat
Copy link
Member

blegat commented Jun 8, 2023

Dualization does not have to define dualization for all sets because for sets for which it's not defined, it's going to be bridged before the dualization layer.
Anyway, checking the number of variables and constraints with dualize wouldn't give what we want. What we should check is the number of variables and constraints after bridging

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

Successfully merging this pull request may close these issues.

3 participants