remove assumption that merged and fixed variables are disjoint #178
Conversation
…t the case they might overflow, fix that
|
I don't really understand this fix. You're suggesting that the "variableCount" can be negative, and that it should be set to 0 instead? We should add a more general fix, as it seems this phenomenon can happen also when variableCount is positive. |
yuvaljacoby
changed the title
|
You are probably right. |
|
For the failing example, did you run in DEBUG mode? Did the assertion at the end of Preprocessor::preprocess go off? |
|
Didn't run in debug mode, didn't know about the DEBUG_ON flag. |
…aljacoby/Marabou-1 into MergedFixedVariablesDisjoint
|
Please try now |
yuvaljacoby
changed the title
|
Works, Thanks. |
|
Great.
Before we merge, please add your previously failing example as either a
unit test for Preprocessor.h, or as a regression test, and see that it
still fails if you comment out the new function call.
…On Mon, 23 Sep 2019 at 09:30, yuvaljacoby ***@***.***> wrote:
Works, Thanks.
Cleaned the pull request, I think we can squash and merge.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<https://github.com/guykatzz/Marabou/pull/178?email_source=notifications&email_token=ADSTPXKYHRCYBEZYT4BZAQTQLBO73A5CNFSM4IY3NBD2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7J4Z7Y#issuecomment-533974271>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADSTPXIBBIHVT7XTPZNUEYTQLBO73ANCNFSM4IY3NBDQ>
.
|
|
done The default compilation of the tests does not have the DEBUG_ON flag, should we change it? |
|
Yes, lets have DBEUG_ON for the tests. |
…lNetworkVerification#178) * we assume the merged and fixed variables are disjoint, but if it's not the case they might overflow, fix that * reproduce problem * add the main test file * an attempted fix * clean * more clean * add test to verify merged and fix variables are disjoint * cosmetics
* we assume the merged and fixed variables are disjoint, but if it's not the case they might overflow, fix that * reproduce problem * add the main test file * an attempted fix * clean * more clean * add test to verify merged and fix variables are disjoint * cosmetics
It's a minor change.
In Preprocessor.cpp we assume that fixedVariables and mergedVariables are disjoint (preprocess function).
I tried an example that it wasn't the case and then number of variables overflowed and the query got stuck.
Not the most elegant solution but fixed my problem...