-
Notifications
You must be signed in to change notification settings - Fork 33
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
Complete model #1019
Complete model #1019
Conversation
cecfab9
to
d9fa809
Compare
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 PR adds a mechanism to keep track of symbols that were declared, and ensure that all declared symbols are defined in generated models. The general design is sound, but I would like the implementation of ModelMap
to be changed to no longer assume that expressions can't be abstract names. The unused DeclSet
functors should also be removed.
src/lib/frontend/d_cnf.ml
Outdated
@@ -53,6 +53,12 @@ module HT = Hashtbl.Make( | |||
let hash = Fun.id | |||
end) | |||
|
|||
module DeclSet = Set.Make | |||
(struct | |||
type t = Id.typed |
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.
Why do we need to use the type here? Can we have multiple declarations with the same name but different types?
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 this is because we need the type in Uf
, this should be a map from identifiers to types rather than a set of typed identifiers.
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.
Actually this doesn't seem to be used.
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.
Sorry, I forgot to remove it ;)
src/lib/frontend/d_cnf.ml
Outdated
let decl = match td with | ||
| `Type_decl (td, _def) -> | ||
mk_ty_decl td; | ||
None | ||
|
||
| `Term_decl td -> | ||
Some (mk_term_decl td) | ||
in | ||
begin match decl with | ||
| Some d -> | ||
let st_loc = dl_to_ael dloc_file loc in | ||
C.{ st_decl = Decl d; st_loc } :: acc | ||
| None -> | ||
acc | ||
end |
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.
No need to build an option
to destruct it immediately:
let decl = match td with | |
| `Type_decl (td, _def) -> | |
mk_ty_decl td; | |
None | |
| `Term_decl td -> | |
Some (mk_term_decl td) | |
in | |
begin match decl with | |
| Some d -> | |
let st_loc = dl_to_ael dloc_file loc in | |
C.{ st_decl = Decl d; st_loc } :: acc | |
| None -> | |
acc | |
end | |
begin match td with | |
| `Type_decl (td, _def) -> | |
mk_ty_decl td; | |
acc | |
| `Term_decl td -> | |
let d = mk_term_decl td in | |
let st_loc = dl_to_ael dloc_file loc in | |
C.{ st_decl = Decl d; st_loc } :: acc | |
end |
src/lib/frontend/d_cnf.ml
Outdated
end | ||
in | ||
aux [] dcl; | ||
acc | ||
aux [] dcl @ acc |
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.
Nit: acc
could be passed as a new 3rd argument to aux
here (to return instead of []
in the final case) so that we avoid building a list just to concatenate it to another one.
src/lib/reasoners/fun_sat.ml
Outdated
declare_ids : Id.typed Vec.t; | ||
declare_lim : int Vec.t; |
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.
There is no need to do this dance in Fun_sat
which is used as a functional data structure, this can simply be a declare_ids : Id.typed list
field that is updated functionally ({ env with declare_ids = id :: env.declare_ids }
) in declare
.
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 was my first implementation and it didn't work. A functional solution would be to use a list of lists:
declare_stack: Id.typed list list
We have to pop/push
this stack in Fun_sat.pop/Fun_sat.push
.
I didn't adopt this solution because I was afraid that a list of lists isn't cache-friendly. I understand that you want to keep the environment of FunSAT
as pure as possible.
Honestly, I believe we should pull up all the imperative stuff of FunSAT
into Funsat_frontend
and use a stack of environment of FunSAT
to simulate the push and the pop functions. In this design, we'll use a vector for declarations in the imperative environment of Funsat_frontend
.
I also tried to implement the feature in Funsat_frontend
without modifying the environment of FunSAT
but we have no access to the right environment of Theory
here.
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.
Ah, right, push
and pop
forces us to be imperative. Oh well. I guess the way to do this while staying consistent with the current Fun_sat
module is Id.typed list Stack.t
rather than Id.typed list list
then.
I didn't adopt this solution because I afraid that a list of lists isn't cache-friendly.
You did write too much C++ :) A list of lists (or a stack of lists, which is the same thing) is very probably not going to be an issue here because we never actually look at the lists until model generation (and also because this only impacts calls to push
and pop
, which is virtually 0% of Alt-Ergo's runtime).
In this design, we'll use a vector for declarations in the imperative environment of Funsat_frontend.
If we use Funsat_frontend
for push
and pop
we don't need a separate vector for declarations in Funsat_frontend
, we can simply have declare_ids : Id.typed list
in Funsat
which will get tracked by the main stack in the frontend, I believe.
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 agree to use Id.typed list Stack.t
:)
src/lib/reasoners/satml_frontend.ml
Outdated
declare_ids : Id.typed Vec.t; | ||
declare_lim : int Vec.t; |
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 would be simpler to use a vector of lists here:
mutable declare_ids : Id.typed list ;
declare_queue : Id.typed list Vec.t ;
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.
Uhm, Id.typed list Vec.t
is a stack, not a queue in your suggestion, right?
I adopt this solution because it's the current style of SatML.
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 it's a stack (for some reasons they are called "queues" elsewhere in satML but we can use the proper teminology ;) ).
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.
So why do we need an extra field declare_ids
here? The top element of the stack can be accessed in O(1)
. I believe
declare_stack: Id.typed list Vec.t
is sufficient.
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.
Even if the algorithmic complexity is identical, the constant term is not the same; having a separate field here avoids having to check that the vector is not empty on all accesses. It probably does not matter here because we never look at the declarations except during model generation but for the other vectors in SatML which are accessed much more frequently it might matter.
It's fine to use a non-empty Vec.t
here (or even a Stack.t
from the stdlib actually).
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 will use the current design with an extra field.
src/lib/structures/modelMap.ml
Outdated
let values = P.add sy (Graph.add arg_vals ret_val graph) values in | ||
{ values; suspicious } | ||
|
||
let empty ~suspicious = { values = P.empty; suspicious } | ||
module DeclSets = Set.Make |
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 doesn't seem used.
src/lib/structures/modelMap.ml
Outdated
it means there is no constraint on this graph. We replace it by | ||
the graph with the only constraint given by [arg_vals] and | ||
[ret_val]. *) | ||
if Graph.cardinal graph == 1 then |
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.
Don't use ==
to compare integers.
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.
Oups, I did too much C++ lately :)
src/lib/structures/modelMap.ml
Outdated
let arg_vals = List.map Expr.mk_abstract arg_tys in | ||
let ret_val = Expr.mk_abstract ret_ty in | ||
let graph = Graph.add arg_vals ret_val Graph.empty in | ||
P.add sy graph values |
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 you are using a graph with a single "dummy" definition here to then detect it in add
and fix things up if the graph actually has definitions. This is quite hacky in part because it introduces a global invariant that abstract values are never otherwise placed into graphs — I believe it is true currently (although I actually had to check in several places to say so) but could easily cause subtle bugs in the future due to unrelated refactorings.
Your suggestion in the PR message to use an ADT (or a record carrying an explicit default value) would be better here; I don't think the extra indirection will be a problem (especially given that it will only impact model generation).
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 know this is a hackish solution. I think the best solution would be to add a function to generate dummy values. If you run cvc5
or z3
on an input file with completely unused symbols, they generate dummy values for them. I didn't do it in this PR because I want a small modification here in order to implement (get-value)
.
Besides, I'm not sure to understand what you mean by a global invariant here. The add
replaces a valid model by another valid model and you don't need to maintain an invariant outside ModelMap
because the only way to add new constraints in your graph is by using ModelMap.add
.
I agree the indirection isn't a big deal.
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'm not sure to understand what you mean by a global invariant here. The add replaces a valid model by another valid model and you don't need to maintain an invariant outside ModelMap because the only way to add new constraints in your graph is by using ModelMap.add.
I mean that in the current implementation it is incorrect to call ModelMap.add
with a ret_val
that is an abstract value:
- The graph is initially
{ [@a0] -> @a1 }
as created byempty
- Adding the mapping
0 -> @x
, the graph becomes{ [0] -> @x }
because it only contains a single abstract value - Adding the mapping
1 -> @y
, the graph becomes{ [1] -> @y }
because it only contains a single abstract value
But this is incorrect and now we will print (define-fun f ((_ Int)) T @y)
but (f 0)
must be @x
, not @y
(abstract values with different names are distinct).
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.
Uhm right, it should be fine with the current state of the code but it's a fragile design. I'll fix it.
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 really determine whether this file is intended to test model generation or the interaction of model generation with incremental solving.
In general it is better to have smaller tests so that it is easier to understand what they are actually testing — for instance, is it important to have x
and z
in the last get-model
here, or are we actually testing model generation for functions?
I'd prefer if you could split this test into smaller files, ideally with a single (check-sat)
per problem file.
(As an aside: are we currently guaranteed to have the same abstract values in two consecutive calls to (get-model)
?)
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.
LGTM — just a couple nits and one question about pop
ping too much but it is not really related to this PR per se.
@@ -177,6 +177,12 @@ module Make (Th : Theory.S) = struct | |||
unit_facts_cache : (E.gformula * Ex.t) ME.t ref; | |||
last_saved_model : Models.t Lazy.t option ref; | |||
unknown_reason : Sat_solver_sig.unknown_reason option; | |||
|
|||
declare_top : Id.typed list ref; |
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.
Nit: unless there is a reason to pass the ref
around, it is usually better to use mutable
record fields rather than ref
s, both for performance (it is slightly more efficient, although as we have established it won't matter here) and to make the intent more clear, in particular that there will be no sharing of the field.
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 was my first attempt of course for sake of performance but it doesn't work. It seems we need to share this field between several environments so we cannot avoid the double indirection with the current implementation.
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.
Hm, that may sound like a bug? We do create new environments (copies) all the time, but the old ones should no longer be used (we overwrite the fields in Fun_sat_frontend
). The pop
issue should also no longer be relevant, because we can write to the mutable field here. Can you elaborate on what "it doesn't work" means?
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 will check what's happened. When I tried using a mutable field instead of a reference, FunSAT keeps declarations of popped assertion levels. Then I switched to a reference (I only did this modification) and it magically worked.
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.
Note that with a mutable field you need to set acc.declare_top <- Stack.pop acc.declare_tail
not env.declare_top <- Stack.pop env.declare_tail
in the pop
function (which I'd guess is the issue given what you describe).
src/lib/reasoners/fun_sat.ml
Outdated
try env.declare_top := Stack.pop env.declare_tail | ||
with Stack.Empty -> | ||
(* Happens if we perform more pops than pushes. *) | ||
invalid_arg "Fun_sat.pop" |
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.
Can this actually happen in practice, or is there some mechanism to catch it before the fact? We shouldn't show a naked exception to the user in this case.
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.
There is no mechanism to catch it if you use the SAT API with the library. But there is a mechanism to prevent it with the binary and this hero is Dolmen :)
I may create a new error in Error
to manage this case?
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.
An exception is appropriate for library use, thanks for checking!
src/lib/structures/modelMap.ml
Outdated
(fun graph -> | ||
match graph with | ||
| C constraints -> C (Constraints.map (subst_in_term id e) constraints) | ||
| Free a -> Free (subst_in_term id e a) |
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.
| Free a -> Free (subst_in_term id e a) | |
| Free _ -> graph |
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 was my first code but let's imagine you're using the library and you get a model with SAT.get_model
. One of the values of the model is an abstract value. You may want to substitute it by something else. Sometimes, it will work because this is not a free graph, sometimes the function subst
silently ignores your request.
My code isn't good because after doing this substitution, the user could add a new constraint to the graph but the current add
function will erase the effect of the previous substitution.
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 model is an output value of the solver, I don't think the use case where we allow people to alter the model after the fact is one that we want to support. If people want to change the model they need to add assertions to the problem and call solve
again to get a new model.
Under that assumption, the semantics of the Free
constructor is "this function is unconstrained", and the inner abstract value is a "dummy" name that is guaranteed to not occur anywhere else in the model (by construction). Given that semantics, we shouldn't substitute under the Free
constructor.
If we want to have a proper semantics for the graphs, the only easy way I see given the current architecture is a record with a default value instead, and print something like (define-fun f (x Int) (ite (= x 0) 0 @a0))
when then only constraint is [0] -> 0
— but again, I don't think we need to do this.
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.
Under that assumption, the semantics of the
Free
constructor is "this function is unconstrained", and the inner abstract value is a "dummy" name that is guaranteed to not occur anywhere else in the model (by construction). Given that semantics, we shouldn't substitute under theFree
constructor.
Note, I don't care either way — I think both implementations are OK as long as we don't expect the user to modify the model. If we do, neither implementation is correct as you point out (but again — I don't think we want to support this).
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 agree we don't need to support it. At some point, we should stop to expose subst
in the library.
4206878
to
7c910bf
Compare
@Halbaroth should we merge this? |
@Halbaroth second ping; are we good to merge or did you want to make additional changes? |
This PR fixes the issue OCamlPro#778. The suggested solution by this PR is as follows: - Collect user-declared symbols in `D_cnf`; - As the function `make` of the module `D_cnf` returns a list of commands, we add a new command `Decl` in `Frontend` to declare symbols; - The declared symbols are given to the SAT solver through a new function `declaration` in `sat_solver_sig`. - These symbols are collected in vectors and we keep indexes in another PR in order to manage properly the push/pop mechanism. - Finally the declared symbols are given to `Uf` and we create the empty model with them. Another annoying point comes from the current datastructure used in `ModelMap`. The type of `ModelMap` saves model values in the form of graphs. A graph is a finite set of constraints but there is no appropriate representation for the graph without constraints. Let's imagine we have the input: ```smt2 (set-option :produce-models true) (set-logic ALL) (declare-fun (Int) Int) (check-sat) (get-model) ``` We expect an output as follows: ```smt2 ( (define-fun f (_x Int) (@A as Int)) ) ``` But `Graph.empty` cannot hold the abstract value `@a`. A naive solution consists in using an ADT: ```ocaml type graph = | Empty of Id.typed | G of Graph.t ``` But we'll add an extra indirection. Probably the best solution would be to use a custom datastructure instead of a map to store the constraints. In this PR, we detect in `ModelMap.add` if the only constraint in the graph is of the form `something -> abstract value`. In this case we remove this constraint. This solution isn't perfect as explained in issue OCamlPro#1018. The biggest drawback of this solution is the fact we have to declare symbols in the SAT API, otherwise models returned by `get_model` could be incomplete.
7c910bf
to
98899b9
Compare
CHANGES: ### Command-line interface - Enable FPA theory by default (OCamlPro/alt-ergo#1177) - Remove deprecated output channels (OCamlPro/alt-ergo#782) - Deprecate the --use-underscore since it produces models that are not SMT-LIB compliant (OCamlPro/alt-ergo#805) - Add --dump-models-on to dump models on a specific output channel (OCamlPro/alt-ergo#838) - Use consistent return codes (OCamlPro/alt-ergo#842) - Add --continue-on-error flag to set the SMT-LIB error behavior (OCamlPro/alt-ergo#853) - Make dolmen the default value for the --frontend option (OCamlPro/alt-ergo#857) - Restore functionality to broken `--profiling` option (OCamlPro/alt-ergo#947) - Add bare-bones support for interactive input in SMT-LIB format (OCamlPro/alt-ergo#949) - Less confusing behavior on timeout (OCamlPro/alt-ergo#982, OCamlPro/alt-ergo#984) - Add `--strict` option for SMT-LIB compliant mode (OCamlPro/alt-ergo#916, OCamlPro/alt-ergo#1133) - Deprecate `sum`, `typing` and `warnings` debug flags (OCamlPro/alt-ergo#1204) ### SMT-LIB support - Add support for the many new SMT-LIB commands (OCamlPro/alt-ergo#837, OCamlPro/alt-ergo#848, OCamlPro/alt-ergo#852, OCamlPro/alt-ergo#863, OCamlPro/alt-ergo#911, OCamlPro/alt-ergo#942, OCamlPro/alt-ergo#945, OCamlPro/alt-ergo#961, OCamlPro/alt-ergo#975, OCamlPro/alt-ergo#1069) - Expose the FPA rounding builtin in the SMT-LIB format (OCamlPro/alt-ergo#876, OCamlPro/alt-ergo#1135) - Allow changing the SAT solver using (set-option) (OCamlPro/alt-ergo#880) - Add support for the `:named` attribute (OCamlPro/alt-ergo#957) - Add support for quoted identifiers (OCamlPro/alt-ergo#909, OCamlPro/alt-ergo#972) - Add support for naming lemmas in SMT-LIB syntax (OCamlPro/alt-ergo#1141, OCamlPro/alt-ergo#1143) ### Model generation - Use post-solve SAT environment in model generation, fixing issues where incorrect models were generated (OCamlPro/alt-ergo#789) - Restore support for records in models (OCamlPro/alt-ergo#793) - Use abstract values instead of dummy values in models where the actual value does not matter (OCamlPro/alt-ergo#804, OCamlPro/alt-ergo#835) - Use fresh names for all abstract values to prevent accidental captures (OCamlPro/alt-ergo#811) - Add support for model generation with the default CDCL solver (OCamlPro/alt-ergo#829) - Support model generation for the BV theory (OCamlPro/alt-ergo#841, OCamlPro/alt-ergo#968) - Add support for optimization (MaxSMT / OptiSMT) with lexicographic Int and Real objectives (OCamlPro/alt-ergo#861, OCamlPro/alt-ergo#921) - Add a SMT-LIB printer for expressions (OCamlPro/alt-ergo#952, OCamlPro/alt-ergo#981, OCamlPro/alt-ergo#1082, OCamlPro/alt-ergo#931, OCamlPro/alt-ergo#932) - Ensure models have a value for all constants in the problem (OCamlPro/alt-ergo#1019) - Fix a rare soundness issue with integer constraints when model generation is enabled (OCamlPro/alt-ergo#1025) - Support model generation for the ADT theory (OCamlPro/alt-ergo#1093, OCamlPro/alt-ergo#1153) ### Theory reasoning - Add word-level propagators for the BV theory (OCamlPro/alt-ergo#944, OCamlPro/alt-ergo#1004, OCamlPro/alt-ergo#1007, OCamlPro/alt-ergo#1010, OCamlPro/alt-ergo#1011, OCamlPro/alt-ergo#1012, OCamlPro/alt-ergo#1040, OCamlPro/alt-ergo#1044, OCamlPro/alt-ergo#1054, OCamlPro/alt-ergo#1055, OCamlPro/alt-ergo#1056, OCamlPro/alt-ergo#1057, OCamlPro/alt-ergo#1065, OCamlPro/alt-ergo#1073, OCamlPro/alt-ergo#1144, OCamlPro/alt-ergo#1152) - Add interval domains and arithmetic propagators for the BV theory (OCamlPro/alt-ergo#1058, OCamlPro/alt-ergo#1083, OCamlPro/alt-ergo#1084, OCamlPro/alt-ergo#1085) - Native support for bv2nat of bit-vector normal forms (OCamlPro/alt-ergo#1154) - Fix incompleteness issues in the BV solver (OCamlPro/alt-ergo#978, OCamlPro/alt-ergo#979) - Abstract more arguments of AC symbols to avoid infinite loops when they contain other AC symbols (OCamlPro/alt-ergo#990) - Do not make irrelevant decisions in CDCL solver, improving performance slightly (OCamlPro/alt-ergo#1041) - Rewrite the ADT theory to use domains and integrate the enum theory into the ADT theory (OCamlPro/alt-ergo#1078, OCamlPro/alt-ergo#1086, OCamlPro/alt-ergo#1087, OCamlPro/alt-ergo#1091, OCamlPro/alt-ergo#1094, OCamlPro/alt-ergo#1138) - Rewrite the Intervals module entirely (OCamlPro/alt-ergo#1108) - Add maximize/minimize terms for matching (OCamlPro/alt-ergo#1166) - Internalize `sign_extend` and `repeat` (OCamlPro/alt-ergo#1192) - Run cross-propagators to completion (OCamlPro/alt-ergo#1221) - Support binary distinct on arbitrary bit-widths (OCamlPro/alt-ergo#1222) - Only perform optimization when building a model (OCamlPro/alt-ergo#1224) - Make sure domains do not overflow the default domain (OCamlPro/alt-ergo#1225) - Do not build unnormalized values in `zero_extend` (OCamlPro/alt-ergo#1226) ### Internal/library changes - Rewrite the Vec module (OCamlPro/alt-ergo#607) - Move printer definitions closer to type definitions (OCamlPro/alt-ergo#808) - Mark proxy names as reserved (OCamlPro/alt-ergo#836) - Use a Zarith-based representation for numbers and bit-vectors (OCamlPro/alt-ergo#850, OCamlPro/alt-ergo#943) - Add native support for (bvnot) in the BV solver (OCamlPro/alt-ergo#856) - Add constant propagators for partially interpreted functions (OCamlPro/alt-ergo#869) - Remove `Util.failwith` in favor of `Fmt.failwith` (OCamlPro/alt-ergo#872) - Add more `Expr` smart constructors (OCamlPro/alt-ergo#877, OCamlPro/alt-ergo#878) - Do not use existential variables for integer division (OCamlPro/alt-ergo#881) - Preserve `Subst` literals to prevent accidental incompleteness (OCamlPro/alt-ergo#886) - Properly start timers (OCamlPro/alt-ergo#924) - Compute a concrete representation of a model instead of performing (OCamlPro/alt-ergo#925) - Allow direct access to the SAT API in the Alt-Ergo library computations during printing (OCamlPro/alt-ergo#927) - Better handling for step limit (OCamlPro/alt-ergo#936) - Add a generic option manager to deal with the dolmen state (OCamlPro/alt-ergo#951) - Expose an imperative SAT API in the Alt-Ergo library (OCamlPro/alt-ergo#962) - Keep track of the SMT-LIB mode (OCamlPro/alt-ergo#971) - Add ability to decide on semantic literals (OCamlPro/alt-ergo#1027, OCamlPro/alt-ergo#1118) - Preserve trigger order when parsing quantifiers with multiple trigger (OCamlPro/alt-ergo#1046). - Store domains inside the union-find module (OCamlPro/alt-ergo#1119) - Remove some polymorphic hashtables (OCamlPro/alt-ergo#1219) ### Build and integration - Drop support for OCaml <4.08.1 (OCamlPro/alt-ergo#803) - Use dune-site for the inequalities plugins. External pluginsm ust now be registered through the dune-site plugin mechanism in the `(alt-ergo plugins)` site to be picked up by Alt-Ergo (OCamlPro/alt-ergo#1049). - Add a release workflow (OCamlPro/alt-ergo#827) - Add a Windows workflow (OCamlPro/alt-ergo#1203) - Mark the dune.inc file as linguist-generated to avoid huge diffs (OCamlPro/alt-ergo#830) - Use GitHub Actions badges in the README (OCamlPro/alt-ergo#882) - Use `dune build @check` to build the project (OCamlPro/alt-ergo#887) - Enable warnings as errors on the CI (OCamlPro/alt-ergo#888) - Uniformization of internal identifiers generation (OCamlPro/alt-ergo#905, OCamlPro/alt-ergo#918) - Use an efficient `String.starts_with` implementation (OCamlPro/alt-ergo#912) - Fix the Makefile (OCamlPro/alt-ergo#914) - Add `Logs` dependency (OCamlPro/alt-ergo#1206) - Use `dynamic_include` to include the generated file `dune.inc` (OCamlPro/alt-ergo#1199) - Support Windows (OCamlPro/alt-ergo#1184,OCamlPro/alt-ergo#1189,OCamlPro/alt-ergo#1195,OCamlPro/alt-ergo#1199,OCamlPro/alt-ergo#1200) - Wrap the library `Alt_ergo_prelude` (OCamlPro/alt-ergo#1223) ### Testing - Use --enable-assertions in tests (OCamlPro/alt-ergo#809) - Add a test for push/pop (OCamlPro/alt-ergo#843) - Use the CDCL solver when testing model generation (OCamlPro/alt-ergo#938) - Do not test .smt2 files with the legacy frontend (OCamlPro/alt-ergo#939) - Restore automatic creation of .expected files (OCamlPro/alt-ergo#941) ### Documentation - Add a new example for model generation (OCamlPro/alt-ergo#826) - Add a Pygments lexer for the Alt-Ergo native language (OCamlPro/alt-ergo#862) - Update the current authors (OCamlPro/alt-ergo#865) - Documentation of the `Enum` theory (OCamlPro/alt-ergo#871) - Document `Th_util.lit_origin` (OCamlPro/alt-ergo#915) - Document the CDCL-Tableaux solver (OCamlPro/alt-ergo#995) - Document Windows support (OCamlPro/alt-ergo#1216) - Remove instructions to install Alt-Ergo on Debian (OCamlPro/alt-ergo#1217) - Document optimization feature (OCamlPro/alt-ergo#1231)
CHANGES: ### Command-line interface - Enable FPA theory by default (OCamlPro/alt-ergo#1177) - Remove deprecated output channels (OCamlPro/alt-ergo#782) - Deprecate the --use-underscore since it produces models that are not SMT-LIB compliant (OCamlPro/alt-ergo#805) - Add --dump-models-on to dump models on a specific output channel (OCamlPro/alt-ergo#838) - Use consistent return codes (OCamlPro/alt-ergo#842) - Add --continue-on-error flag to set the SMT-LIB error behavior (OCamlPro/alt-ergo#853) - Make dolmen the default value for the --frontend option (OCamlPro/alt-ergo#857) - Restore functionality to broken `--profiling` option (OCamlPro/alt-ergo#947) - Add bare-bones support for interactive input in SMT-LIB format (OCamlPro/alt-ergo#949) - Less confusing behavior on timeout (OCamlPro/alt-ergo#982, OCamlPro/alt-ergo#984) - Add `--strict` option for SMT-LIB compliant mode (OCamlPro/alt-ergo#916, OCamlPro/alt-ergo#1133) - Deprecate `sum`, `typing` and `warnings` debug flags (OCamlPro/alt-ergo#1204) ### SMT-LIB support - Add support for the many new SMT-LIB commands (OCamlPro/alt-ergo#837, OCamlPro/alt-ergo#848, OCamlPro/alt-ergo#852, OCamlPro/alt-ergo#863, OCamlPro/alt-ergo#911, OCamlPro/alt-ergo#942, OCamlPro/alt-ergo#945, OCamlPro/alt-ergo#961, OCamlPro/alt-ergo#975, OCamlPro/alt-ergo#1069) - Expose the FPA rounding builtin in the SMT-LIB format (OCamlPro/alt-ergo#876, OCamlPro/alt-ergo#1135) - Allow changing the SAT solver using (set-option) (OCamlPro/alt-ergo#880) - Add support for the `:named` attribute (OCamlPro/alt-ergo#957) - Add support for quoted identifiers (OCamlPro/alt-ergo#909, OCamlPro/alt-ergo#972) - Add support for naming lemmas in SMT-LIB syntax (OCamlPro/alt-ergo#1141, OCamlPro/alt-ergo#1143) ### Model generation - Use post-solve SAT environment in model generation, fixing issues where incorrect models were generated (OCamlPro/alt-ergo#789) - Restore support for records in models (OCamlPro/alt-ergo#793) - Use abstract values instead of dummy values in models where the actual value does not matter (OCamlPro/alt-ergo#804, OCamlPro/alt-ergo#835) - Use fresh names for all abstract values to prevent accidental captures (OCamlPro/alt-ergo#811) - Add support for model generation with the default CDCL solver (OCamlPro/alt-ergo#829) - Support model generation for the BV theory (OCamlPro/alt-ergo#841, OCamlPro/alt-ergo#968) - Add support for optimization (MaxSMT / OptiSMT) with lexicographic Int and Real objectives (OCamlPro/alt-ergo#861, OCamlPro/alt-ergo#921) - Add a SMT-LIB printer for expressions (OCamlPro/alt-ergo#952, OCamlPro/alt-ergo#981, OCamlPro/alt-ergo#1082, OCamlPro/alt-ergo#931, OCamlPro/alt-ergo#932) - Ensure models have a value for all constants in the problem (OCamlPro/alt-ergo#1019) - Fix a rare soundness issue with integer constraints when model generation is enabled (OCamlPro/alt-ergo#1025) - Support model generation for the ADT theory (OCamlPro/alt-ergo#1093, OCamlPro/alt-ergo#1153) ### Theory reasoning - Add word-level propagators for the BV theory (OCamlPro/alt-ergo#944, OCamlPro/alt-ergo#1004, OCamlPro/alt-ergo#1007, OCamlPro/alt-ergo#1010, OCamlPro/alt-ergo#1011, OCamlPro/alt-ergo#1012, OCamlPro/alt-ergo#1040, OCamlPro/alt-ergo#1044, OCamlPro/alt-ergo#1054, OCamlPro/alt-ergo#1055, OCamlPro/alt-ergo#1056, OCamlPro/alt-ergo#1057, OCamlPro/alt-ergo#1065, OCamlPro/alt-ergo#1073, OCamlPro/alt-ergo#1144, OCamlPro/alt-ergo#1152) - Add interval domains and arithmetic propagators for the BV theory (OCamlPro/alt-ergo#1058, OCamlPro/alt-ergo#1083, OCamlPro/alt-ergo#1084, OCamlPro/alt-ergo#1085) - Native support for bv2nat of bit-vector normal forms (OCamlPro/alt-ergo#1154) - Fix incompleteness issues in the BV solver (OCamlPro/alt-ergo#978, OCamlPro/alt-ergo#979) - Abstract more arguments of AC symbols to avoid infinite loops when they contain other AC symbols (OCamlPro/alt-ergo#990) - Do not make irrelevant decisions in CDCL solver, improving performance slightly (OCamlPro/alt-ergo#1041) - Rewrite the ADT theory to use domains and integrate the enum theory into the ADT theory (OCamlPro/alt-ergo#1078, OCamlPro/alt-ergo#1086, OCamlPro/alt-ergo#1087, OCamlPro/alt-ergo#1091, OCamlPro/alt-ergo#1094, OCamlPro/alt-ergo#1138) - Rewrite the Intervals module entirely (OCamlPro/alt-ergo#1108) - Add maximize/minimize terms for matching (OCamlPro/alt-ergo#1166) - Internalize `sign_extend` and `repeat` (OCamlPro/alt-ergo#1192) - Run cross-propagators to completion (OCamlPro/alt-ergo#1221) - Support binary distinct on arbitrary bit-widths (OCamlPro/alt-ergo#1222) - Only perform optimization when building a model (OCamlPro/alt-ergo#1224) - Make sure domains do not overflow the default domain (OCamlPro/alt-ergo#1225) - Do not build unnormalized values in `zero_extend` (OCamlPro/alt-ergo#1226) ### Internal/library changes - Rewrite the Vec module (OCamlPro/alt-ergo#607) - Move printer definitions closer to type definitions (OCamlPro/alt-ergo#808) - Mark proxy names as reserved (OCamlPro/alt-ergo#836) - Use a Zarith-based representation for numbers and bit-vectors (OCamlPro/alt-ergo#850, OCamlPro/alt-ergo#943) - Add native support for (bvnot) in the BV solver (OCamlPro/alt-ergo#856) - Add constant propagators for partially interpreted functions (OCamlPro/alt-ergo#869) - Remove `Util.failwith` in favor of `Fmt.failwith` (OCamlPro/alt-ergo#872) - Add more `Expr` smart constructors (OCamlPro/alt-ergo#877, OCamlPro/alt-ergo#878) - Do not use existential variables for integer division (OCamlPro/alt-ergo#881) - Preserve `Subst` literals to prevent accidental incompleteness (OCamlPro/alt-ergo#886) - Properly start timers (OCamlPro/alt-ergo#924) - Compute a concrete representation of a model instead of performing (OCamlPro/alt-ergo#925) - Allow direct access to the SAT API in the Alt-Ergo library computations during printing (OCamlPro/alt-ergo#927) - Better handling for step limit (OCamlPro/alt-ergo#936) - Add a generic option manager to deal with the dolmen state (OCamlPro/alt-ergo#951) - Expose an imperative SAT API in the Alt-Ergo library (OCamlPro/alt-ergo#962) - Keep track of the SMT-LIB mode (OCamlPro/alt-ergo#971) - Add ability to decide on semantic literals (OCamlPro/alt-ergo#1027, OCamlPro/alt-ergo#1118) - Preserve trigger order when parsing quantifiers with multiple trigger (OCamlPro/alt-ergo#1046). - Store domains inside the union-find module (OCamlPro/alt-ergo#1119) - Remove some polymorphic hashtables (OCamlPro/alt-ergo#1219) ### Build and integration - Drop support for OCaml <4.08.1 (OCamlPro/alt-ergo#803) - Use dune-site for the inequalities plugins. External pluginsm ust now be registered through the dune-site plugin mechanism in the `(alt-ergo plugins)` site to be picked up by Alt-Ergo (OCamlPro/alt-ergo#1049). - Add a release workflow (OCamlPro/alt-ergo#827) - Add a Windows workflow (OCamlPro/alt-ergo#1203) - Mark the dune.inc file as linguist-generated to avoid huge diffs (OCamlPro/alt-ergo#830) - Use GitHub Actions badges in the README (OCamlPro/alt-ergo#882) - Use `dune build @check` to build the project (OCamlPro/alt-ergo#887) - Enable warnings as errors on the CI (OCamlPro/alt-ergo#888) - Uniformization of internal identifiers generation (OCamlPro/alt-ergo#905, OCamlPro/alt-ergo#918) - Use an efficient `String.starts_with` implementation (OCamlPro/alt-ergo#912) - Fix the Makefile (OCamlPro/alt-ergo#914) - Add `Logs` dependency (OCamlPro/alt-ergo#1206) - Use `dynamic_include` to include the generated file `dune.inc` (OCamlPro/alt-ergo#1199) - Support Windows (OCamlPro/alt-ergo#1184,OCamlPro/alt-ergo#1189,OCamlPro/alt-ergo#1195,OCamlPro/alt-ergo#1199,OCamlPro/alt-ergo#1200) - Wrap the library `Alt_ergo_prelude` (OCamlPro/alt-ergo#1223) ### Testing - Use --enable-assertions in tests (OCamlPro/alt-ergo#809) - Add a test for push/pop (OCamlPro/alt-ergo#843) - Use the CDCL solver when testing model generation (OCamlPro/alt-ergo#938) - Do not test .smt2 files with the legacy frontend (OCamlPro/alt-ergo#939) - Restore automatic creation of .expected files (OCamlPro/alt-ergo#941) ### Documentation - Add a new example for model generation (OCamlPro/alt-ergo#826) - Add a Pygments lexer for the Alt-Ergo native language (OCamlPro/alt-ergo#862) - Update the current authors (OCamlPro/alt-ergo#865) - Documentation of the `Enum` theory (OCamlPro/alt-ergo#871) - Document `Th_util.lit_origin` (OCamlPro/alt-ergo#915) - Document the CDCL-Tableaux solver (OCamlPro/alt-ergo#995) - Document Windows support (OCamlPro/alt-ergo#1216) - Remove instructions to install Alt-Ergo on Debian (OCamlPro/alt-ergo#1217) - Document optimization feature (OCamlPro/alt-ergo#1231)
This PR fixes the issue #778. The suggested solution by this PR is as follows:
D_cnf
;make
of the moduleD_cnf
returns a list ofcommands, we add a new command
Decl
inFrontend
to declaresymbols;
function
declaration
insat_solver_sig
;vector in order to manage properly the push/pop mechanism.
Uf
and we create the emptymodel with them.
Another annoying point comes from the current data structure used in
ModelMap
. The type ofModelMap
saves model values in the form of graphs.A graph is a finite set of constraints but there is no appropriate
representation for the graph without constraints. Let's imagine we have
the input:
We expect an output as follows:
But
Graph.empty
cannot hold the abstract value@a
. A naive solutionconsists in using an ADT:
But we'll add an extra indirection. Probably the best solution would be
to use a custom data structure instead of a map to store the constraints.
In this PR, we detect in
ModelMap.add
if the only constraint in thegraph is of the form
something -> abstract value
. In this case weremove this constraint.
This solution isn't perfect as explained in issue #1018. The biggest
drawback of this solution is the fact we have to declare symbols in the
SAT API, otherwise models returned by
get_model
could beincomplete.