-
Notifications
You must be signed in to change notification settings - Fork 35
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
Skolemization issue using constrain in query #407
Comments
Good catch. In fact, the issue isn't that skolemization isn't working (it actually is), but access from the query portion to any skolemized variable is something we cannot support. Consider: issue :: Symbolic (Bool, Bool)
issue = do
x :: SBool <- forall_
y :: SBool <- exists_
constrain y
query $ do checkSat
(,) <$> getValue x <*> getValue y This requests both
The right thing to do is to recognize if there are quantifiers and reject user queiries if they are present. @bts Before I implement this (and reject the query dynamically), I want to double check: Is there an actual use case here that we should consider, or is this a mere curioisty? (i.e., is recognizing and rejecting these cases with an error message acceptable?) |
Thanks a lot for looking into this. There is indeed a use-case, but I suspect there's probably another way that we can achieve our goal. Ultimately the primary motivation is that @joelburget and I are trying to reuse work with push & pop ( Before this refactor, we had been using See this (very rough) pseudocode: results = runSMTWith z3 $ do
analyzeProgram program -- this accumulates constraints along the way
preds <- analyzeProperties properties -- these can use any quantifiers. this never calls 'constrain'
query $
for preds $ \pred ->
inNewAssertionStack $ do
-- Note that we can't use 'output' here. Also this is the problematic 'constrain'
-- in the face of quantifiers:
constrain $ bnot pred
queryResult -- uses checkSat and extracts a falsifying model, if applicable If it were possible to use results = runSMTWith z3 $ do
analyzeProgram program -- this accumulates constraints along the way
preds <- analyzeProperties properties -- these can use any quantifiers. this never calls 'constrain'
query $
for preds $ \pred ->
inNewAssertionStack $ do
-- this output would be "rolled back" between stack frames. normally instead
-- of manually negating the output, we'd use valid (vs sat mode), but let's ignore that:
output $ bnot pred
queryResult -- uses checkSat and extracts a falsifying model, if applicable While we're using stacks now, we would like to introduce constraint vacuity checking if possible. We can add this to either of the two above approaches, but we notice that if we add it to our latter/ results = runSMTWith z3 $ do
analyzeProgram program -- this accumulates constraints along the way
preds <- analyzeProperties properties -- these can use any quantifiers. this never calls 'constrain'
query $
for preds $ \pred -> do
inNewAssertionStack $ do
-- this 'constrain' is problematic in the face of quantifiers:
constrain pred
checkConstraintVacuity -- uses checkSat
inNewAssertionStack $ do
-- this output would be "rolled back" between stack frames:
output $ bnot pred
queryResult -- uses checkSat and extracts a falsifying model, if applicable Thank you again for all of your help! It's very much appreciated. |
Unfortunately, I don't see an easy/safe way to support both user queries and quantifiers at the same time. The issue is rather historical: Support for quantifiers in SBV has always been such that they scoped over the entire program. In the early days, this was the easiest thing to do. In hindsight, however, this isn't quite right: When quantifiers are used in SMTLib, they are usually intended for small delimited assertions. The scoping rules we have for quantifiers just don't match their intended use case. I'm not sure how to go about solving your issue; but I'd question if quantifiers are really necessary. Would be best to avoid them if at all possible. An isolated replicable example might help clarify. In the meantime, I will go ahead and make the changes to reject user queries if quantifiers are present. This is the safe thing to do, though I do understand it doesn't really help you at all. |
@bts We now catch and reject these cases:
I know this isn't ideal for you, but I feel it's the right thing to do. When you have an isolated use case, please report that as a seperate ticket and see if we can somehow relax this restriction. Till that time, I'm inclined to reject all quantification if we go into user queries. Thanks for reporting. |
Thanks Levent. I agree that this is the right thing to do. In our case, unfortunately we are nearly certain that quantifiers are necessary. It's okay though; we can survive without query mode, and if we need speed-ups, we can try resorting to concurrency. Thanks again for your help! |
This is temporarily disabled until we have some resolution on this SBV issue. See: LeventErkok/sbv#407
Hi Levent,
I have some code that was working fine until I moved a
constrain
from outside, to inside, the use ofquery
.It seems likely that the problem is related to skolemization bookkeeping?
If I move the
constrain
back outside ofquery
, things work as expected:I can reproduce this for both sbv 7.8 and HEAD.
Thanks,
Brian
The text was updated successfully, but these errors were encountered: