Skip to content

Conversation

blegat
Copy link
Member

@blegat blegat commented Jul 15, 2021

Needed by ECOS since it needs a StructOfConstraints with MOI.Zeros on one side and Union{MOI.Nonnegatives,MOI.SecondOrderCone,MOI.ExponentialCone} on the other side.

MOIU.@struct_of_constraints_by_set_types(
ZerosOrNot,
MOI.Zeros,
Union{MOI.Nonnegatives,MOI.Nonpositives},
Copy link
Member

Choose a reason for hiding this comment

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

Can you clarify why this is needed in more detail?!? Why not just two separate fields.

Copy link
Member Author

Choose a reason for hiding this comment

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

This allows to have one sparse matrix with the two instead of two separate sparse matrices. ECOS needs one sparse matrix with the three non-Zeros cones

Copy link
Member

Choose a reason for hiding this comment

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

Sure, but you could concatenate the sparse matrices? It seems weird that we go to all this effort and then allow Union

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd argue that this PR is simpler than implementing concatenation in the solver wrapper though. DiffOpt also needs to split the matrix onto equality and non-equality. This is quite common so having this feature is not just for ECOS

@blegat blegat merged commit 971e430 into master Jul 16, 2021
@blegat blegat deleted the bl/struct_of_cons_union branch July 16, 2021 11:34
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.

2 participants