Make theory preprocess rewrite equalities a preprocessing pass (#5711)
authorAndrew Reynolds <andrew.j.reynolds@gmail.com>
Tue, 22 Dec 2020 14:24:24 +0000 (08:24 -0600)
committerGitHub <noreply@github.com>
Tue, 22 Dec 2020 14:24:24 +0000 (08:24 -0600)
Some theories rewrite equalities during ppRewrite. An example is arithmetic with the option arith-rewrite-eq, which rewrites (= x y) to (and (>= x y) (<= x y)) during theory preprocessing.

This PR makes it so that ppRewrite is only called on equalities in preprocessing, during a new preprocessing pass "TheoryRewriteEq". On the other hand, ppRewrite is never called on new equalities generated in lemmas from TheoryEngine.

In detail, the motivation for this change:

(1) Rewriting equalities during ppRewrite is dangerous since it may break invariants wrt theory combination. In particular, equalities in splitting lemmas originating from theory combination must not be theory-preprocessed, or else we may be non-terminating or solution unsound. This can happen if a theory requests a split on (= x y) but is not notified of this atom when another theory rewrites (= x y) during ppRewrite.

(2) After this PR, we can simplify our policy for all lemmas generated, in particular, we can say that all lemmas must be theory preprocessed before their literals are asserted to TheoryEngine. This is now possible as the invariant cannot be broken (theoryRewriteEq is relegated to the preprocessor, which is only applied once). This will make LemmaProperty::PREPROCESS obsolete, which in turn will simplify several lemma caches for nonlinear and quantifiers. It will also significantly simplify proof production for the theory preprocessor (which maintains two stacks of utilities for preprocessed vs non-preprocessed lemmas).

(3) Simplifications to the above policy will make it significantly easier to implement theory-preprocessing apply when literals are asserted. It is currently not possible to implement this in a coherent way without tracking which literals were a part of lemmas marked as "do not theory-preprocess".

13 files changed:
src/CMakeLists.txt
src/preprocessing/passes/theory_rewrite_eq.cpp [new file with mode: 0644]
src/preprocessing/passes/theory_rewrite_eq.h [new file with mode: 0644]
src/preprocessing/preprocessing_pass_registry.cpp
src/smt/process_assertions.cpp
src/theory/arith/theory_arith.cpp
src/theory/theory.h
src/theory/theory_engine.cpp
src/theory/theory_engine.h
src/theory/theory_preprocessor.cpp
test/regress/regress1/quantifiers/qbv-simple-2vars-vo.smt2
test/regress/regress1/quantifiers/qbv-subcall.smt2
test/regress/regress1/quantifiers/smtcomp-qbv-053118.smt2

index 82e88f330d29fe1b0e6276746c19feb5edec844a..0fb50582346b5ea0be4cc60f970da5d373836909 100644 (file)
@@ -108,6 +108,8 @@ libcvc4_add_sources(
   preprocessing/passes/synth_rew_rules.h
   preprocessing/passes/theory_preprocess.cpp
   preprocessing/passes/theory_preprocess.h
+  preprocessing/passes/theory_rewrite_eq.cpp
+  preprocessing/passes/theory_rewrite_eq.h
   preprocessing/passes/unconstrained_simplifier.cpp
   preprocessing/passes/unconstrained_simplifier.h
   preprocessing/preprocessing_pass.cpp
diff --git a/src/preprocessing/passes/theory_rewrite_eq.cpp b/src/preprocessing/passes/theory_rewrite_eq.cpp
new file mode 100644 (file)
index 0000000..68006b6
--- /dev/null
@@ -0,0 +1,113 @@
+/*********************                                                        */
+/*! \file theory_rewrite_eq.cpp
+ ** \verbatim
+ ** Top contributors (to current version):
+ **   Andrew Reynolds
+ ** This file is part of the CVC4 project.
+ ** Copyright (c) 2009-2020 by the authors listed in the file AUTHORS
+ ** in the top-level source directory and their institutional affiliations.
+ ** All rights reserved.  See the file COPYING in the top-level source
+ ** directory for licensing information.\endverbatim
+ **
+ ** \brief The TheoryRewriteEq preprocessing pass
+ **/
+
+#include "preprocessing/passes/theory_rewrite_eq.h"
+
+#include "theory/theory_engine.h"
+
+using namespace CVC4::theory;
+
+namespace CVC4 {
+namespace preprocessing {
+namespace passes {
+
+TheoryRewriteEq::TheoryRewriteEq(PreprocessingPassContext* preprocContext)
+    : PreprocessingPass(preprocContext, "theory-rewrite-eq"){};
+
+PreprocessingPassResult TheoryRewriteEq::applyInternal(
+    AssertionPipeline* assertions)
+{
+  // apply ppRewrite to all equalities in assertions
+  for (std::size_t i = 0, size = assertions->size(); i < size; ++i)
+  {
+    Node assertion = (*assertions)[i];
+    TrustNode trn = rewriteAssertion(assertion);
+    if (!trn.isNull())
+    {
+      // replace based on the trust node
+      assertions->replaceTrusted(i, trn);
+    }
+  }
+  return PreprocessingPassResult::NO_CONFLICT;
+}
+
+theory::TrustNode TheoryRewriteEq::rewriteAssertion(TNode n)
+{
+  NodeManager* nm = NodeManager::currentNM();
+  TheoryEngine* te = d_preprocContext->getTheoryEngine();
+  std::unordered_map<TNode, Node, TNodeHashFunction> visited;
+  std::unordered_map<TNode, Node, TNodeHashFunction>::iterator it;
+  std::vector<TNode> visit;
+  TNode cur;
+  visit.push_back(n);
+  do
+  {
+    cur = visit.back();
+    visit.pop_back();
+    it = visited.find(cur);
+
+    if (it == visited.end())
+    {
+      if (cur.getKind() == kind::EQUAL)
+      {
+        // For example, (= x y) ---> (and (>= x y) (<= x y))
+        theory::TrustNode trn = te->ppRewriteEquality(cur);
+        // can make proof producing by using proof generator from trn
+        visited[cur] = trn.isNull() ? Node(cur) : trn.getNode();
+      }
+      else
+      {
+        visited[cur] = Node::null();
+        visit.push_back(cur);
+        visit.insert(visit.end(), cur.begin(), cur.end());
+      }
+    }
+    else if (it->second.isNull())
+    {
+      Node ret = cur;
+      bool childChanged = false;
+      std::vector<Node> children;
+      if (cur.getMetaKind() == kind::metakind::PARAMETERIZED)
+      {
+        children.push_back(cur.getOperator());
+      }
+      for (const Node& cn : cur)
+      {
+        it = visited.find(cn);
+        Assert(it != visited.end());
+        Assert(!it->second.isNull());
+        childChanged = childChanged || cn != it->second;
+        children.push_back(it->second);
+      }
+      if (childChanged)
+      {
+        ret = nm->mkNode(cur.getKind(), children);
+      }
+      visited[cur] = ret;
+    }
+  } while (!visit.empty());
+  Assert(visited.find(n) != visited.end());
+  Assert(!visited.find(n)->second.isNull());
+  Node ret = visited[n];
+  if (ret == n)
+  {
+    return TrustNode::null();
+  }
+  // can make proof producing by providing a term conversion generator here
+  return TrustNode::mkTrustRewrite(n, ret, nullptr);
+}
+
+}  // namespace passes
+}  // namespace preprocessing
+}  // namespace CVC4
diff --git a/src/preprocessing/passes/theory_rewrite_eq.h b/src/preprocessing/passes/theory_rewrite_eq.h
new file mode 100644 (file)
index 0000000..bc40273
--- /dev/null
@@ -0,0 +1,56 @@
+/*********************                                                        */
+/*! \file theory_rewrite_eq.h
+ ** \verbatim
+ ** Top contributors (to current version):
+ **   Andrew Reynolds
+ ** This file is part of the CVC4 project.
+ ** Copyright (c) 2009-2020 by the authors listed in the file AUTHORS
+ ** in the top-level source directory and their institutional affiliations.
+ ** All rights reserved.  See the file COPYING in the top-level source
+ ** directory for licensing information.\endverbatim
+ **
+ ** \brief The TheoryRewriteEq preprocessing pass
+ **/
+
+#include "cvc4_private.h"
+
+#ifndef CVC4__PREPROCESSING__PASSES__THEORY_REWRITE_EQ_H
+#define CVC4__PREPROCESSING__PASSES__THEORY_REWRITE_EQ_H
+
+#include "preprocessing/preprocessing_pass.h"
+#include "preprocessing/preprocessing_pass_context.h"
+#include "theory/trust_node.h"
+
+namespace CVC4 {
+namespace preprocessing {
+namespace passes {
+
+/**
+ * Implements the preprocessing pass for called ppRewrite on all equalities
+ * in the input. This is required to be a preprocessing pass since it is not
+ * recommended that ppRewrite is called on equalities generated in lemmas (e.g.
+ * it may interfere with equality splitting in theory combination).
+ */
+class TheoryRewriteEq : public PreprocessingPass
+{
+ public:
+  TheoryRewriteEq(PreprocessingPassContext* preprocContext);
+
+ protected:
+  PreprocessingPassResult applyInternal(
+      AssertionPipeline* assertionsToPreprocess) override;
+  /**
+   * Rewrite the assertion based on rewriting equalities based on theory
+   * specific rewriting.
+   * An example is removing arithmetic equalities via:
+   *   (= x y) ---> (and (>= x y) (<= x y))
+   * Returns the trust node corresponding to the rewrite.
+   */
+  theory::TrustNode rewriteAssertion(TNode assertion);
+};
+
+}  // namespace passes
+}  // namespace preprocessing
+}  // namespace CVC4
+
+#endif /* CVC4__PREPROCESSING__PASSES__THEORY_REWRITE_EQ_H */
index 46c1fff681e995db27cfe23e4eb4268b7181a471..b21d4e30c9065a76448c7fa3a52f9a41e10aeddd 100644 (file)
@@ -53,6 +53,7 @@
 #include "preprocessing/passes/sygus_inference.h"
 #include "preprocessing/passes/synth_rew_rules.h"
 #include "preprocessing/passes/theory_preprocess.h"
+#include "preprocessing/passes/theory_rewrite_eq.h"
 #include "preprocessing/passes/unconstrained_simplifier.h"
 #include "preprocessing/preprocessing_pass.h"
 
@@ -149,6 +150,7 @@ PreprocessingPassRegistry::PreprocessingPassRegistry()
   registerPassInfo("bool-to-bv", callCtor<BoolToBV>);
   registerPassInfo("ho-elim", callCtor<HoElim>);
   registerPassInfo("fun-def-fmf", callCtor<FunDefFmf>);
+  registerPassInfo("theory-rewrite-eq", callCtor<TheoryRewriteEq>);
 }
 
 }  // namespace preprocessing
index 8b9b0a53ce0740c66b062f6271e04a01f45975ff..7fd1db7971c0ef5fe5e659724dfc8687c5e2b083 100644 (file)
@@ -317,6 +317,9 @@ bool ProcessAssertions::apply(Assertions& as)
     d_passes["ho-elim"]->apply(&assertions);
   }
 
+  // rewrite equalities based on theory-specific rewriting
+  d_passes["theory-rewrite-eq"]->apply(&assertions);
+
   // begin: INVARIANT to maintain: no reordering of assertions or
   // introducing new ones
 
index d1dd4bb63ed6b79076a549b58ce34c0c3e114567..7437ce223f053459e2751b655c84cd8cbbecbe8b 100644 (file)
@@ -112,8 +112,9 @@ TrustNode TheoryArith::ppRewrite(TNode atom)
 
   if (options::arithRewriteEq())
   {
-    if (atom.getKind() == kind::EQUAL && atom[0].getType().isReal())
+    if (atom.getKind() == kind::EQUAL)
     {
+      Assert(atom[0].getType().isReal());
       Node leq = NodeBuilder<2>(kind::LEQ) << atom[0] << atom[1];
       Node geq = NodeBuilder<2>(kind::GEQ) << atom[0] << atom[1];
       TrustNode tleq = ppRewriteTerms(leq);
@@ -138,10 +139,7 @@ TrustNode TheoryArith::ppRewrite(TNode atom)
 
 TrustNode TheoryArith::ppRewriteTerms(TNode n)
 {
-  if (Theory::theoryOf(n) != THEORY_ARITH)
-  {
-    return TrustNode::null();
-  }
+  Assert(Theory::theoryOf(n) == THEORY_ARITH);
   // Eliminate operators recursively. Notice we must do this here since other
   // theories may generate lemmas that involve non-standard operators. For
   // example, quantifier instantiation may use TO_INTEGER terms; SyGuS may
index 6256360e4c6adb71085b6c1846b02e14fee07294..a9783c19c1f9a5c0e09b96b0e4f339f47f8272b5 100644 (file)
@@ -721,15 +721,20 @@ class Theory {
                                   TrustSubstitutionMap& outSubstitutions);
 
   /**
-   * Given an atom of the theory coming from the input formula, this
-   * method can be overridden in a theory implementation to rewrite
-   * the atom into an equivalent form.  This is only called just
-   * before an input atom to the engine. This method returns a TrustNode of
-   * kind TrustNodeKind::REWRITE, which carries information about the proof
-   * generator for the rewrite. Similarly to expandDefinition, this method may
-   * return the null TrustNode if atom is unchanged.
-   */
-  virtual TrustNode ppRewrite(TNode atom) { return TrustNode::null(); }
+   * Given a term of the theory coming from the input formula or
+   * from a lemma generated during solving, this method can be overridden in a
+   * theory implementation to rewrite the term into an equivalent form.
+   *
+   * This method returns a TrustNode of kind TrustNodeKind::REWRITE, which
+   * carries information about the proof generator for the rewrite, which can
+   * be the null TrustNode if n is unchanged.
+   *
+   * Notice this method is used both in the "theory rewrite equalities"
+   * preprocessing pass, where n is an equality from the input formula,
+   * and in theory preprocessing, where n is a (non-equality) term occurring
+   * in the input or generated in a lemma.
+   */
+  virtual TrustNode ppRewrite(TNode n) { return TrustNode::null(); }
 
   /**
    * Notify preprocessed assertions. Called on new assertions after
index 941d900d3baa0ebe62b9960c72a6dcc2f47eeb9e..5206ed7aee6e00611b3191f72b1282c06b73e604 100644 (file)
@@ -852,6 +852,12 @@ theory::Theory::PPAssertStatus TheoryEngine::solve(
   return solveStatus;
 }
 
+theory::TrustNode TheoryEngine::ppRewriteEquality(TNode eq)
+{
+  Assert(eq.getKind() == kind::EQUAL);
+  return theoryOf(eq)->ppRewrite(eq);
+}
+
 void TheoryEngine::notifyPreprocessedAssertions(
     const std::vector<Node>& assertions) {
   // call all the theories
index bade5a20604c38e10b7b0d35f36774ffb5a0f720..41f8372a9363ab865bc29d536d2906d45e23396b 100644 (file)
@@ -439,7 +439,11 @@ class TheoryEngine {
   bool isProofEnabled() const;
 
  public:
-
+  /**
+   * Preprocess rewrite equality, called by the preprocessor to rewrite
+   * equalities appearing in the input.
+   */
+  theory::TrustNode ppRewriteEquality(TNode eq);
   /** Notify (preprocessed) assertions. */
   void notifyPreprocessedAssertions(const std::vector<Node>& assertions);
 
index 7c917dff5e8397abbb9825072d1c66a54bd2173e..c56554b53ef364e5a2575a22bdb4533b3ac5b28d 100644 (file)
@@ -450,6 +450,23 @@ Node TheoryPreprocessor::preprocessWithProof(Node term)
   // be steps from the same term to multiple rewritten forms, which would be
   // the case if we registered a preprocessing step for a non-rewritten term.
   Assert(term == Rewriter::rewrite(term));
+  // We never call ppRewrite on equalities here, since equalities have a
+  // special status. In particular, notice that theory preprocessing can be
+  // called on all formulas asserted to theory engine, including those generated
+  // as new literals appearing in lemmas. Calling ppRewrite on equalities is
+  // incompatible with theory combination where a split on equality requested
+  // by a theory could be preprocessed to something else, thus making theory
+  // combination either non-terminating or result in solution soundness.
+  // Notice that an alternative solution is to ensure that certain lemmas
+  // (e.g. splits from theory combination) can never have theory preprocessing
+  // applied to them. However, it is more uniform to say that theory
+  // preprocessing is applied to all formulas. This makes it so that e.g.
+  // theory solvers do not need to specify whether they want their lemmas to
+  // be theory-preprocessed or not.
+  if (term.getKind() == kind::EQUAL)
+  {
+    return term;
+  }
   // call ppRewrite for the given theory
   TrustNode trn = d_engine.theoryOf(term)->ppRewrite(term);
   if (trn.isNull())
index 31cc963252d3f24ce93d75d1c81bb884a953c9ba..f291ec0ec47271bb2d8c5fdf4955af28e867b2f3 100644 (file)
@@ -1,4 +1,4 @@
-; COMMAND-LINE: --cegqi-bv --cegqi-bv-ineq=keep
+; COMMAND-LINE: --cegqi-bv --cegqi-bv-ineq=keep -q
 ; EXPECT: sat
 (set-logic BV)
 (set-info :status sat)
index 5d6881411dc0710f039279b1efa53ac84dab2ad1..42a93921bebd1743f98583f0d846f107958f0ca3 100644 (file)
@@ -1,3 +1,5 @@
+; COMMAND-LINE: -q
+; EXPECT: sat
 (set-logic BV)
 (set-info :status sat)
 (declare-fun k_141 () (_ BitVec 16))
index 6cbe4db5c4ac5e97b447bbe19d75456fbca255f3..d8bb5fcaf3e5f0ef3ee53e7b7e48b6f4772e502e 100644 (file)
@@ -1,3 +1,5 @@
+; COMMAND-LINE: -q
+; EXPECT: sat
 (set-info :smt-lib-version 2.6)
 (set-logic BV)
 (set-info :status sat)