-
Notifications
You must be signed in to change notification settings - Fork 52
Add Proper Yices2 Quantifier Support #447
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
Changes from 10 commits
acaf7da
3e318c0
3d4b036
25be000
64a70f7
9a256e9
a787c54
d84820f
efc25cf
96aeb4d
ac51adb
3f456a8
5dcc40d
6f8c6c6
c08797a
dbdd97a
95daa09
fd268cc
025c460
62ab126
72728c0
fb94b95
9faf82d
af010b6
1a09bf6
09ac268
e700fb7
43d0140
fb5d3e5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,7 @@ | |
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.YICES_DIVIDES_ATOM; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.YICES_EQ_TERM; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.YICES_FLOOR; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.YICES_FORALL_TERM; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.YICES_IDIV; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.YICES_IMOD; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.YICES_IS_INT_ATOM; | ||
|
@@ -81,6 +82,7 @@ | |
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_eq; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_floor; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_function_type; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_get_term_by_name; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_get_term_name; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_idiv; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_imod; | ||
|
@@ -89,7 +91,8 @@ | |
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_is_int_atom; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_ite; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_mul; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_named_variable; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_new_uninterpreted_term; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_new_variable; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_not; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_or; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_parse_rational; | ||
|
@@ -100,6 +103,7 @@ | |
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_proj_index; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_rational_const_value; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_real_type; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_set_term_name; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_sum; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_sum_component; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_term_bitsize; | ||
|
@@ -120,9 +124,11 @@ | |
import com.google.common.base.Joiner; | ||
import com.google.common.base.Preconditions; | ||
import com.google.common.collect.Collections2; | ||
import com.google.common.collect.HashBasedTable; | ||
import com.google.common.collect.ImmutableList; | ||
import com.google.common.collect.ImmutableSet; | ||
import com.google.common.collect.Lists; | ||
import com.google.common.collect.Table; | ||
import com.google.common.primitives.Ints; | ||
import java.math.BigInteger; | ||
import java.util.ArrayList; | ||
|
@@ -135,6 +141,7 @@ | |
import org.sosy_lab.java_smt.api.FormulaType; | ||
import org.sosy_lab.java_smt.api.FormulaType.FloatingPointType; | ||
import org.sosy_lab.java_smt.api.FunctionDeclarationKind; | ||
import org.sosy_lab.java_smt.api.QuantifiedFormulaManager.Quantifier; | ||
import org.sosy_lab.java_smt.api.visitors.FormulaVisitor; | ||
import org.sosy_lab.java_smt.basicimpl.FormulaCreator; | ||
import org.sosy_lab.java_smt.basicimpl.FunctionDeclarationImpl; | ||
|
@@ -153,6 +160,12 @@ public class Yices2FormulaCreator extends FormulaCreator<Integer, Integer, Long, | |
YICES_VARIABLE, | ||
YICES_UNINTERPRETED_TERM); | ||
|
||
/** | ||
* Maps a name and a free variable or function type to a concrete formula node. We allow only 1 | ||
* type per var name, meaning there is only 1 column per row! | ||
*/ | ||
private final Table<String, Integer, Integer> formulaCache = HashBasedTable.create(); | ||
|
||
protected Yices2FormulaCreator() { | ||
super(null, yices_bool_type(), yices_int_type(), yices_real_type(), null, null); | ||
} | ||
|
@@ -174,7 +187,7 @@ public Integer getArrayType(Integer pIndexType, Integer pElementType) { | |
|
||
@Override | ||
public Integer makeVariable(Integer pType, String pVarName) { | ||
return yices_named_variable(pType, pVarName); | ||
return createNamedVariable(pType, pVarName); | ||
} | ||
|
||
@Override | ||
|
@@ -246,6 +259,60 @@ public FormulaType<?> getFormulaType(Integer pFormula) { | |
yices_type_to_string(yices_type_of_term(pFormula)), yices_term_to_string(pFormula))); | ||
} | ||
|
||
/** Creates a named, free variable. Might retrieve it from the cache if created prior. */ | ||
protected int createNamedVariable(int type, String name) { | ||
Integer maybeFormula = formulaCache.get(name, type); | ||
if (maybeFormula != null) { | ||
return maybeFormula; | ||
} | ||
if (formulaCache.containsRow(name)) { | ||
throw new IllegalArgumentException( | ||
"Symbol " + name + " already used for a variable of type " + formulaCache.row(name)); | ||
} | ||
int var = yices_new_uninterpreted_term(type); | ||
// Names in Yices2 behave like a stack. The last variable named is retrieved when asking for | ||
// a term with a specific name. Since we substitute free vars with bound for quantifiers, | ||
// this sometimes mixes them up, hence we track them ourselves. | ||
yices_set_term_name(var, name); | ||
formulaCache.put(name, type, var); | ||
return var; | ||
} | ||
|
||
protected int createBoundVariableFromFreeVariable(int unboundVar) { | ||
int type = yices_type_of_term(unboundVar); | ||
String name = yices_get_term_name(unboundVar); | ||
|
||
int termFromName = yices_get_term_by_name(name); | ||
if (termFromName != -1) { | ||
int termFromNameType = yices_type_of_term(termFromName); | ||
if (type == termFromNameType) { | ||
int constructor = yices_term_constructor(termFromName); | ||
if (constructor == YICES_VARIABLE) { | ||
// Already a bound var | ||
return termFromName; | ||
} | ||
} else { | ||
throw new IllegalArgumentException( | ||
String.format( | ||
"Can't create variable with name '%s' and type '%s' " | ||
+ "as it would omit a variable with type '%s'", | ||
name, yices_type_to_string(type), yices_type_to_string(termFromNameType))); | ||
} | ||
} | ||
// reset term name binding | ||
// TODO: add yices_remove_term_name(); | ||
int bound = yices_new_variable(type); | ||
// Names in Yices2 behave like a stack. The last variable named is retrieved when asking for | ||
// a term with a specific name. Since we substitute free vars with bound for quantifiers, | ||
// this sometimes mixes them up, hence we track them ourselves. | ||
// This overrides the naming, but the old is cached. | ||
// Meaning that if we remove the new name, the old term gets its name back. | ||
// Since we just want to retrieve the same var for the quantifier we are currently building, | ||
// this is fine. | ||
yices_set_term_name(bound, name); | ||
return bound; | ||
} | ||
|
||
@Override | ||
public <R> R visit(FormulaVisitor<R> pVisitor, Formula pFormula, Integer pF) { | ||
int constructor = yices_term_constructor(pF); | ||
|
@@ -258,6 +325,22 @@ public <R> R visit(FormulaVisitor<R> pVisitor, Formula pFormula, Integer pF) { | |
return pVisitor.visitConstant(pFormula, convertValue(pF, pF)); | ||
case YICES_UNINTERPRETED_TERM: | ||
return pVisitor.visitFreeVariable(pFormula, yices_get_term_name(pF)); | ||
case YICES_VARIABLE: | ||
return pVisitor.visitBoundVariable(pFormula, 0); | ||
case YICES_FORALL_TERM: | ||
int children = yices_term_num_children(pF); | ||
ImmutableList.Builder<Formula> boundVars = ImmutableList.builder(); | ||
for (int i = 0; i < children - 1; i++) { | ||
int boundVar = yices_term_child(pF, i); | ||
boundVars.add(encapsulateWithTypeOf(boundVar)); | ||
} | ||
BooleanFormula body = encapsulateBoolean(yices_term_child(pF, children - 1)); | ||
Quantifier quant = Quantifier.EXISTS; | ||
if (yices_term_to_string(pF).startsWith("(forall")) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yices is open source. We could find out, how Yices computes the string and check for the flag that tells the difference from FORALL to EXISTS: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yices does not seem to have a separate term for
To see if we have a
I couldn't find any public API for this, but we could probably just keep track of the polarity in the visitor. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No actually that is the solution! Thank you very much! This opens another questions however: do we traverse and/or rebuild terms correctly with this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (I fear that the visitor might rebuild a formula There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exactly as i expected, this happens. I added a test that should work for all solvers that checks this for both types of quantifiers. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK so Yices2 does simply transform the formula on-the-fly while traversing it. Do we want to accept this behavior @kfriedberger ? |
||
// TODO: this is stupid, but i could not find a way of discerning between the quantifiers | ||
quant = Quantifier.FORALL; | ||
} | ||
return pVisitor.visitQuantifier((BooleanFormula) pFormula, quant, boundVars.build(), body); | ||
default: | ||
return visitFunctionApplication(pVisitor, pFormula, pF, constructor); | ||
} | ||
|
@@ -696,7 +779,7 @@ public Integer declareUFImpl(String pName, Integer pReturnType, List<Integer> pA | |
} else { | ||
yicesFuncType = yices_function_type(size, argTypeArray, pReturnType); | ||
} | ||
int uf = yices_named_variable(yicesFuncType, pName); | ||
int uf = createNamedVariable(yicesFuncType, pName); | ||
return uf; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,6 +46,7 @@ | |
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_free_config; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_free_context; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_function_type; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_get_term_by_name; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_get_term_name; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_idiv; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_iff; | ||
|
@@ -54,7 +55,6 @@ | |
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_int64; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_int_type; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_mul; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_named_variable; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_new_config; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_new_context; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_new_uninterpreted_term; | ||
|
@@ -81,6 +81,8 @@ | |
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_term_num_children; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_term_to_string; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_true; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_type_of_term; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_type_to_string; | ||
import static org.sosy_lab.java_smt.solvers.yices2.Yices2NativeApi.yices_zero_extend; | ||
|
||
import com.google.common.base.Joiner; | ||
|
@@ -95,6 +97,7 @@ | |
import org.junit.AssumptionViolatedException; | ||
import org.junit.Before; | ||
import org.junit.BeforeClass; | ||
import org.junit.Ignore; | ||
import org.junit.Test; | ||
import org.sosy_lab.common.NativeLibraries; | ||
import org.sosy_lab.common.rationals.Rational; | ||
|
@@ -201,6 +204,8 @@ public void rationalError() { | |
System.out.println(rat); // "use" variable | ||
} | ||
|
||
// TODO: what is this test supposed to be doing? And remove print. | ||
@Ignore | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most of those tests are simply tests for loading and running Yices. They serves as examples and were helpful for debugging initial issues. If the tests do no longer bring benefit, please remove them, and do not just disable them. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I reworked all tests with printouts into assertions and re-enabled all. I also found some issues and fixed them, added documentation and 2 more native tests. |
||
@Test | ||
public void negativeRationalError() { | ||
// TODO negative unsigned integer causes no error. Need to ensure positive value before | ||
|
@@ -467,6 +472,8 @@ public void arithSimplification() { | |
assertThat(yices_term_constructor(mul)).isEqualTo(YICES_ARITH_CONST); | ||
} | ||
|
||
// TODO: what is this test supposed to be doing? And remove print. | ||
@Ignore | ||
@Test | ||
public void sumComponents() { | ||
int three = yices_int32(3); | ||
|
@@ -499,6 +506,8 @@ public void sumComponents() { | |
} | ||
} | ||
|
||
// TODO: what is this test supposed to be doing? And remove print. | ||
@Ignore | ||
@Test | ||
public void bvSumComponents() { | ||
int bv1 = yices_parse_bvbin("00101"); | ||
|
@@ -521,6 +530,8 @@ public void bvSumComponents() { | |
} | ||
} | ||
|
||
// TODO: what is this test supposed to be doing? And remove print. | ||
@Ignore | ||
@Test | ||
public void bvExtensionStructureTest() { | ||
int extendBy = 5; | ||
|
@@ -569,6 +580,8 @@ public void bvSum() { | |
assertThat(constructor).isEqualTo(YICES_BV_SUM); | ||
} | ||
|
||
// TODO: what is this test supposed to be doing? And remove print. | ||
@Ignore | ||
@Test | ||
public void bvMul() { | ||
int type = yices_bv_type(5); | ||
|
@@ -580,4 +593,27 @@ public void bvMul() { | |
System.out.println(component[1]); | ||
System.out.println(yices_term_constructor(yices_bvpower(component[0], component[1]))); | ||
} | ||
|
||
/** | ||
* Only to be used for tests in this class. Old implementation used for creating/retrieving named | ||
* variables. Superseded by Yices2FormulaCreator.createNamedVariable() for reasons outlined there. | ||
*/ | ||
private static int yices_named_variable(int type, String name) { | ||
int termFromName = yices_get_term_by_name(name); | ||
if (termFromName != -1) { | ||
int termFromNameType = yices_type_of_term(termFromName); | ||
if (type == termFromNameType) { | ||
return termFromName; | ||
} else { | ||
throw new IllegalArgumentException( | ||
String.format( | ||
"Can't create variable with name '%s' and type '%s' " | ||
+ "as it would omit a variable with type '%s'", | ||
name, yices_type_to_string(type), yices_type_to_string(termFromNameType))); | ||
} | ||
} | ||
int var = yices_new_uninterpreted_term(type); | ||
yices_set_term_name(var, name); | ||
return var; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This IF-THEN-ELSE logic seems to be used twice. Can we refactor it into a proper utility method? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only parts of the structure is reused. This version is for tests and for free variables only. The other one is for bound variables and has additional logic. (I reworked the creation of free vars and its not similar to this one anymore) |
||
} |
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 some ELSE-cases are missing. Is this intentional?
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 is intentional. I added documentation for it.
Essentially: Names work like a stack in Yices2. If we associate a term with a name, we get that term if we ask yices_get_term_by_name(). However, if we create bound variables, we associate them with the same name as the free variable (so that it has the same name). This pushes the name stack, and we get the bound var when asking yices_get_term_by_name(). That's why i added a cache for free vars. So this way we can re-use bound variables, and don't clash with free vars.