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 Variable bridges to use runtests without unbridged_variable #2498

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

blegat
Copy link
Member

@blegat blegat commented May 10, 2024

Some variable bridges don't support unbridged_variable and that makes runtests fail when getting ConstraintFunction or when getting ConstraintSet for scalar constraints. For example, ZerosBridge does not support it so it cannot use runtests.
Another example is the SOS cone: jump-dev/SumOfSquares.jl#353 I could support it in the nonweighted version but if it's weighted by arbitrary polynomials, it seems difficult to unbridge. It might be possible but it's not high priority. By using only vector cones in the tests, it turns out that only this small change is enough to pass the tests. This is still testing the inner model completely.

  • Explain in docstring why this is used for
  • Use it for ZerosBridge by adding a tests without scalar sets

_test_structural_identical(
test,
model;
allow_constraint_function_error = allow_outer_constraint_function_error,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
allow_constraint_function_error = allow_outer_constraint_function_error,
allow_constraint_function_error,

This should be sufficient instead of the long line?

Copy link
Member Author

Choose a reason for hiding this comment

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

Note the outer_. I only want to allow it when comparing test with model, not with inner

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'm open to better names ^^

Copy link
Member

Choose a reason for hiding this comment

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

I, in fact, did not notice the _outer_ 😆

@odow
Copy link
Member

odow commented May 12, 2024

Did you try ZerosBridge? It still failed in a bunch of places

@blegat
Copy link
Member Author

blegat commented May 13, 2024

I did not try, I was just hoping it could serve as a test

@odow
Copy link
Member

odow commented May 13, 2024

The issue is that a bunch of other places may call ConstraintFunction, so it is not so simple

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.

None yet

2 participants