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
QEL: Fast Approximated Quantifier Elimination #6820
Conversation
no change to code
Partial array equality, PEQ, is used as an intermediate expression during MBP for arrays. We need to factor it out so that it can be shared between MBP-QEL and existing MBP. Partial array equality (peq) is used in MBP for arrays. Factoring this out to be used by multiple MBP implementations.
These rules are specializes for terms that are created in QEL. QEL commit is comming later
The rule handles this special case: (cons (head x) (tail x)) --> x
Special rules to simplify PEQs
Spacer prfers to not have a term representing default value of an array. This guides IUC from picking such terms in interpolation
Class properties allow to keep information for an equivalence class. Getters and setters for terms allow accessing information
QEL commit is comming later in the history
Reperesent equalities explicitly by nodes in the term_graph
This changes the default projection engine that spacer uses.
New commands are mbp-qel -- MBP with term graphs qel -- QEL with term graphs qe-lite -- older qelite
The Mac Build keeps failing on c_example.exe On the build machine we get:
Locally on windows I get (in a "qel" fork):
|
Some build warnings on my Linux code space
|
|
|
return res; | ||
|
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.
2 -> 4
src/qe/mbp/mbp_solve_plugin.cpp
Outdated
if (!is_pos) { | ||
res = mk_not(m, res); | ||
} | ||
if(try_int_mul_solve(atom, is_pos, res)) |
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.
if( -> if (
src/qe/mbp/mbp_solve_plugin.cpp
Outdated
|
||
if (!a.is_int(lhs)) { return false; } | ||
|
||
if(!a.is_mul(rhs)) { |
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.
if( -> if (
src/qe/mbp/mbp_solve_plugin.cpp
Outdated
// returns `true` if a rewritting happened | ||
bool try_int_mul_solve(expr *atom, bool is_pos, expr_ref &res) { | ||
|
||
if (!is_pos) |
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.
2 -> 4
#include "ast/ast.h" | ||
#include "model/model.h" | ||
#include "util/params.h" | ||
|
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.
use C++ format for class declaration. See other headers where "public" keyword appears
src/qe/mbp/mbp_qel.cpp
Outdated
add_vars(p, vars); | ||
} | ||
} | ||
} while (progress); |
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.
} \n while (progress);
src/qe/mbp/mbp_dt_tg.h
Outdated
#include "util/obj_hashtable.h" | ||
|
||
namespace mbp { | ||
class mbp_dt_tg: public mbp_tg_plugin { |
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.
indentation is not consistent
@@ -0,0 +1,88 @@ | |||
#include "ast/array_peq.h" |
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.
header
could you also update RELEASE_NOTES.md? |
Fixed this. One of our rewrites was not general enough to be put in the theory rewriter. Will update PR. |
@@ -21,33 +21,8 @@ Module Name: | |||
br_status datatype_rewriter::mk_app_core(func_decl * f, unsigned num_args, expr * const * args, expr_ref & result) { | |||
SASSERT(f->get_family_id() == get_fid()); | |||
switch(f->get_decl_kind()) { | |||
case OP_DT_CONSTRUCTOR: { |
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.
datatype_rewriter is used from other code paths than QEL.
Why would this code be deleted?
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.
because it is unsound?
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.
yes. The rewriter rewrites cons(head(x), tail(x))
into x
. This is sound only if is_cons(x)
. In our MBP, we meet this precondition. In general, this is unsound
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 latest commit (0e78d24) moves the rewrite into qe_mbp
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.
oh, it is not in master branch anyway, or was removed
@@ -9,6 +9,7 @@ Version 4.next | |||
- polysat | |||
- native word level bit-vector solving. | |||
- introduction of simple induction lemmas to handle a limited repertoire of induction proofs. | |||
- Light quantifier elimination based on term graphs (egraphs), and corresponding Model Based Projection for arrays and ADTs. Used by Spacer and QSAT. |
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.
Mention the new parameter smt.mbp_qsat_qel and what effect it has on using QEL (in qsat?). Elsewhere QEL is always on? And the command mbp-qel.
@@ -9,6 +9,7 @@ Version 4.next | |||
- polysat | |||
- native word level bit-vector solving. | |||
- introduction of simple induction lemmas to handle a limited repertoire of induction proofs. | |||
- Light quantifier elimination based on term graphs (egraphs), and corresponding Model Based Projection for arrays and ADTs. Used by Spacer and QSAT. | |||
|
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 looks like qel is used to replace qe_lite in spacer, but otherwise qe-lite is exposed as usual as a tactic.
Nowadays I would create a "simplifier" (in ast/simplifiers) that invokes qe functionality such that it can be used incrementally and as a tactic. There is a simple wrapper that takes a simplifier and creates a tactic.
It should be relatively easy because qel preserves equivalence. It can be disabled if proof production is required.
Note that if qel was exposed as a tactic maybe the custom commands are not as critical for testing.
If the result of applying tactics from command-line isn't sufficient for use scenarios, we may wrap such differently. For example, we have the "simplify" command that takes an expression and returns an expression. A version for tactics are applied on expressions to expressions could be created if this is really what is needed for evaluation or command-line use.
Are we really going to have both qe-lite and qel?
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.
In the medium term, we would like to see qe-lite be replaced with qel.
In the short term, there are still some problems that behave better with qe-lite. It is convenient to have qe-lite available for debugging and comparison until we resolve these issues as either bug fixes or expected anomalies.
It would be useful for us to switch all external users to qel to see that it is not creating any new or unexpected issues in the use-cases we have not tested for.
Do you want us to expose qel as a tactic similar to qe-lite for now?
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.
I can't say for sure what to do in short term: the interface to qel requires at least some wrapping to create a simplifier. The wrapping could be through a rewriter where the method for handling quantifiers is changed. So rewriter, simplifier, tactic.
Not sure it is worth writing special case code for this when it really is a quantifier rewriter which applies also to the der rewriter, and qe-lite.
qe_light_tactic.cpp contains some qel namespace but not the calls into term graphs from what I can tell.
Light quantifier elimination based on term graphs (egraphs), and corresponding Model Based Projection for arrays and ADTs. Used by Spacer and QSAT.
Isabel Garcia-Contreras, Hari Govind V. K., Sharon Shoham, Arie Gurfinkel: Fast Approximations of Quantifier Elimination. Computer-Aided Verification (CAV). 2023. URL: https://arxiv.org/abs/2306.10009