-
Notifications
You must be signed in to change notification settings - Fork 54
Model usage: fix usage of model values and model evaluation after changing the underlying context. #553
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
base: master
Are you sure you want to change the base?
Conversation
…nging the underlying context. Several solvers referenced a live prover context in the model, allowing the user to insert further queries in the model or remove queries from the context, potentially making the model inconsistent and unreliable for further iteration or evaluation. The solution consists of several steps, for some solvers: - compute the value-assignments upfront when constructing the model. - invalidate the model once the context is changed. Solvers that already provide a persistent model, such as MathSAT, SMTInterpol, and Z3, will not change their behaviour. However, other solvers will throw an exception on invalid usage. While this change is backward-compatible in the API, it is a serious bugfix that changes the behaviour of some solver bindings in JavaSMT, because we need to loosen a basic guarantee for models.
baierd
left a comment
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.
Nice addition/cleanup! Thank you!
2 requests to extend the cleanup even more, nothing mayor.
| * returned <code>false</code>. | ||
| */ | ||
| @Override | ||
| public List<BooleanFormula> getUnsatCore() { |
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.
UnsatCores can also affect solver states (e.g. in Z3). It might be a good idea to generally assume that they do?
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 you give an example? Why would Z3 change its state when calling getUnsatCore?
src/org/sosy_lab/java_smt/solvers/boolector/BoolectorAbstractProver.java
Outdated
Show resolved
Hide resolved
daniel-raffler
left a comment
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.
Looks good to me!
…and unsat core. This fixes a invalid return value in Z3OptimizationProver: We wrongly returned "false" instead of throwing "UnsupportedOperationException".
…, even on empty stack.
Several solvers referenced a live prover context in the model, allowing the user to insert further queries in the model or remove queries from the context, potentially making the model inconsistent and unreliable for further iteration or evaluation.
The solution consists of several steps, for some solvers:
Solvers that already provide a persistent model, such as MathSAT, SMTInterpol, and Z3, will not change their behaviour.
However, other solvers will throw an exception on invalid usage.
While this change is backward-compatible in the API, it is a serious bugfix that changes the behaviour of some solver bindings in JavaSMT, because we need to loosen a basic guarantee for models.