-
Notifications
You must be signed in to change notification settings - Fork 98
Add a check that bridge is used #2811
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
Conversation
I don't understand this. Can you point to an example? |
| [z, 2.0 * x * x] in Indicator{ACTIVATE_ON_ONE}(LessThan(2.0)) | ||
| z in ZeroOne() | ||
| """, | ||
| no_bridge_used = true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one has an explicit comment that the model is unchanged. Is this PR really needed?
|
When you don't implement support properly for the bridge then the bridge isn't used and you get complicated errors saying that the constraint types of the expected model don't match. You might think that the expected model you wrote is wrong or that your bridging isn't done correctly until you realize that the bridge just isn't being used. I thought that starting with a test that says : hey you bridge isn't even being used helps you gain time in many cases. |
|
This has just never happened to me 😄 |
6a1fdcc to
703af5a
Compare
| maxobjective: [x] | ||
| """, | ||
| """; | ||
| no_bridge_used = true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also technically a breaking change because the tests fail without this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's only breaking for tests so it won't really break user code. And this breakage would be quite rare
|
I think I want to close this as won't fix. |
Most of the time when I debug a new bridge, it starts by not being used and I sometimes loose some time checking it. Having it explicitly checked would be helpful.