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.

Sign up for GitHub

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

Jump to bottom

[Bridges.Constraint] add bridges in alphabetical order #2596

Merged
merged 9 commits into from
Jan 5, 2025
Merged

[Bridges.Constraint] add bridges in alphabetical order #2596

merged 9 commits into from
Jan 5, 2025

Conversation

odow
Copy link
Member

@odow odow commented Dec 21, 2024

No description provided.

This was referenced Dec 30, 2024
Merged
Merged
@odow
Copy link
Member Author

odow commented Dec 30, 2024

I pulled out the .Variable stuff into #2599

I'll update this to focus only on .Constraint

@odow odow force-pushed the od/constraint-bridges branch 2 times, most recently from a74fddc to 950a540 Compare December 30, 2024 20:16
@odow odow changed the title [Bridges] explicitly document bridges not added by default [Bridges.Constraint] add bridges in alphabetical order Dec 30, 2024
@odow
Copy link
Member Author

odow commented Dec 30, 2024

https://github.com/jump-dev/MathOptInterface.jl/actions/runs/12552291548

@odow
Copy link
Member Author

odow commented Jan 1, 2025

https://github.com/jump-dev/MathOptInterface.jl/actions/runs/12573879685

@odow odow force-pushed the od/constraint-bridges branch from c498e19 to f0abce0 Compare January 2, 2025 22:18
@blegat
Copy link
Member

blegat commented Jan 5, 2025

It's much cleaner like this, if we desire a priority over bridges, we should rely in explicit bridge cost, not bridge order. It also make sense that the bridges that we needed to add cost for are these soc to sdp ones.

odow
odow commented Jan 5, 2025
View reviewed changes
test/Bridges/lazy_bridge_optimizer.jl
@@ -1751,13 +1750,13 @@ function test_bridge_selection()
bridged_mock,
MOI.VectorOfVariables,
MOI.RotatedSecondOrderCone,
) == MOI.Bridges.Constraint.RSOCtoPSDBridge{
) == MOI.Bridges.Constraint.RSOCtoSOCBridge{
Copy link
Member Author

Choose a reason for hiding this comment

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

@blegat this test should have shown us the problem! It bridged RSOC to PSD, even though RSOC to SOC was available. It used to pick toPSD because that was the first bridge that was added.

@odow odow merged commit 33ec305 into master Jan 5, 2025
15 of 16 checks passed
@odow odow deleted the od/constraint-bridges branch January 5, 2025 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blegat blegat blegat left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@odow @blegat