-
Notifications
You must be signed in to change notification settings - Fork 18
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
heuristic based boolean sat solving #109
Conversation
Still WIP
Further concerns
|
The unit test is failed by enumerating to the candidate of:
However, the It seems that during random enumeration. EMakeMaxTreeMultiset is used to replace TBag — but this seems wrong. I would believe that it is another bug, related to the treemultiset implementation I’ve done before. However, I don't why this bug is revealed only with the current change (which seems remotely related). |
Also, notice that there are a few changes that set |
cf407c6
to
b0c2539
Compare
Is this still in progress? I won't take a close look at the source code until you are ready. Here are a few notes that might save you some time: Cozy requires the solver to produce "complete" examples. A complete example includes a binding for every variable in the context, even if that variable's value does not affect satisfiability, and even if that variable is not in the given formula. I would expect Currently, Cozy communicates the set of variables to the solver module when it constructs the solver object:
There are two possible reasons.
I know for sure that (2) is true. (1) might be true also! If you address the complete-examples point above, I think we'll have a better idea of whether there is a bug.
I think you are right, the TreeSet bug is unrelated. I can guess at a reason: sometimes Cozy's behavior is affected by which counterexamples it sees. The counterexamples affect which expressions Cozy throws out as duplicates during synthesis. There is probably some very complicated explanation for this. |
Thanks for taking a look at this!
I think it is ready for review and it has already fulfilled the functionality I want it to, despite triggering other bugs. Also, since it is a heuristic and designed optional, I might not implement some unsupported features such as uninterpreted function.
I am still confused -- why is that necessary? |
Complete examples are not strictly necessary. However, there is a tradeoff. I had to choose between:
I opted for (1). To illustrate how confusing incomplete examples can be: how would you compare them for equality? Consider these two examples:
Are they equal? In one sense no: the first one does not constrain the value for When I was writing the solver/evaluator/synthesis code, I thought it would be simpler to require complete examples everywhere. The solver has an extra responsibility, but there are fewer surprises everywhere else. |
I will take a look at the code here when I get a chance in the evening. :) |
b0c2539
to
a00102e
Compare
Updated, thanks for the comments! |
(NOTE: This is still the remotely related problem mentioned above....) How to reproduce the errors as in CI:
Also, the error msg:
|
Also, I found an interesting case from the failed unit test that for expression
I stared at it for a while and still believe that random assignment does satisfy that boolean expression. What do you think @Calvin-L ? |
a00102e
to
563f830
Compare
You are right, that is a satisfying assignment. However, for me, the solver is able to find it. Do you have a test case that exhibits the problem? Here's the code I threw together: from cozy.target_syntax import *
from cozy.syntax_tools import pprint, mk_lambda
from cozy.typecheck import retypecheck
from cozy.solver import satisfy
xs = EVar("xs").with_type(BOOL_BAG)
var12 = EVar("_var12").with_type(BOOL)
e = ENot(EEq(
EFilter(EStateVar(xs), ELambda(var12, var12)),
EStateVar(xs)))
assert retypecheck(e)
print(pprint(e))
print(satisfy(e)) Output:
It's possible that the solver returned As a performance optimization, since assumptions are static for the duration of a synthesis job, the assumptions are set when the solver is created. They are not part of the formula passed to In general, these two expressions should be equivalent: solver_for_context(ctx, assumptions).satisfy(e) satisfy(EAll([assumptions, e]),
vars=[v for v, _ in context.vars()],
funcs=context.funcs()) You may want to pass the assumptions to |
Ah, this is very helpful! I caught that when I set a breakpoint during the synthesis loop -- let me work on it a bit more |
@Calvin-L regarding the |
563f830
to
e061006
Compare
e061006
to
d39abae
Compare
@Calvin-L Do you have time to take a look at this? |
tests/random_assignment.py
Outdated
(EBinOp(EUnaryOp('sum', EStateVar(EVar('l').with_type(TBag(TInt()))).with_type(TBag(TInt()))).with_type(TInt()), '+', EVar('i').with_type(TInt())).with_type(TInt()), ENum(0).with_type(TInt()), {'l': Bag((0, -2438, 2437)), 'i': 0}), | ||
(EBinOp(EBinOp(EFilter(EStateVar(EVar('l').with_type(TBag(TInt()))).with_type(TBag(TInt())), ELambda(EVar('_var372').with_type(TInt()), EBinOp(EVar('_var372').with_type(TInt()), '>', ENum(10).with_type(TInt())).with_type(TBool()))).with_type(TBag(TInt())), '+', ECond(EBinOp(EVar('i').with_type(TInt()), '>', ENum(10).with_type(TInt())).with_type(TBool()), ESingleton(EVar('i').with_type(TInt())).with_type(TBag(TInt())), EEmptyList().with_type(TBag(TInt()))).with_type(TBag(TInt()))).with_type(TBag(TInt())), '-', EStateVar(EFilter(EVar('l').with_type(TBag(TInt())), ELambda(EVar('_var492').with_type(TInt()), EBinOp(EVar('_var492').with_type(TInt()), '>', ENum(10).with_type(TInt())).with_type(TBool()))).with_type(TBag(TInt()))).with_type(TBag(TInt()))).with_type(TBag(TInt())), EBinOp(EBinOp(EFilter(EStateVar(EVar('l').with_type(TBag(TInt()))).with_type(TBag(TInt())), ELambda(EVar('_var372').with_type(TInt()), EBinOp(EVar('_var372').with_type(TInt()), '>', ENum(10).with_type(TInt())).with_type(TBool()))).with_type(TBag(TInt())), '+', ECond(EBool(True).with_type(TBool()), ESingleton(EVar('i').with_type(TInt())).with_type(TBag(TInt())), EEmptyList().with_type(TBag(TInt()))).with_type(TBag(TInt()))).with_type(TBag(TInt())), '-', EStateVar(EFilter(EVar('l').with_type(TBag(TInt())), ELambda(EVar('_var492').with_type(TInt()), EBinOp(EVar('_var492').with_type(TInt()), '>', ENum(10).with_type(TInt())).with_type(TBool()))).with_type(TBag(TInt()))).with_type(TBag(TInt()))).with_type(TBag(TInt())), {'l': Bag(()), 'i': 0}), | ||
(EBinOp(EUnaryOp('sum', EStateVar(EVar('l').with_type(TBag(TInt()))).with_type(TBag(TInt()))).with_type(TInt()), '+', EVar('i').with_type(TInt())).with_type(TInt()), EUnaryOp('sum', EStateVar(EVar('l').with_type(TBag(TInt() |
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.
Minor nit that you may or may not care about: the context object lists all the variables that the solver should care about. It so happens that the solver allows additional variables in the formula (i.e. the free variables in the test cases above). However, I don't know if we want to guarantee that behavior. A free variable in a formula that is not present in the context may indicate an internal bug, so we might change the solver in the future to throw an exception in this case.
A "more correct" thing would be to construct a context containing a superset of each test case's free variables. However, what you have here does work today.
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.
(The comment above refers to the init_solver
function; I accidentally attached it to the wrong lines!)
@@ -30,7 +31,7 @@ | |||
from cozy.typecheck import is_collection, is_scalar | |||
from cozy.syntax_tools import subst, pprint, free_vars, fresh_var, alpha_equivalent, strip_EStateVar, freshen_binders, wrap_naked_statevars, break_conj, inline_lets | |||
from cozy.wf import exp_wf | |||
from cozy.common import No, OrderedSet, unique, OrderedSet, StopException | |||
from cozy.common import No, unique, OrderedSet, StopException |
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.
Thanks for fixing this import 👍
Test: $ cd examples $ make select ... _var758 : Bag<(Int, Int, String, Int)> = FlatMap {r -> FlatMap {s -> FlatMap {q -> Map {w -> ((r).A, (s).C, (q).B, (w).C)} (Filter {w -> (((q).B == (w).B) and ((r).A == 15))} (Ws))} (Qs)} (Ss)} (Rs) return _var758 ...
d39abae
to
0a92890
Compare
I've rebased the change and added a new issue regarding our discussion. |
Description
the goal is to speed up the counterexample finding for deciding whether
target
andnew_target
is equivalent. We used to do it in SAT solver, but this could be extremely slow when given a deeply nestedflatMap
desugared from list comprehension expression. In that case, we can exploit conditions clauses and make smarter random assignments that satisfy the refutation easierThis aims to solve #108
How to test
This means that Cozy successfully applies heuristic in determining that it can lift the list comprehension from query to state (since in this particular example there is no update to it).