Make substitution index context-independent (#2474)
authorAndres Noetzli <andres.noetzli@gmail.com>
Fri, 27 Sep 2019 17:57:58 +0000 (10:57 -0700)
committerGitHub <noreply@github.com>
Fri, 27 Sep 2019 17:57:58 +0000 (10:57 -0700)
When we do solving in incremental mode, we store substitutions at a
special index in our list of assertions. Previously, we used a
context-dependent variable for that. However, this is not needed since
the list of assertions just consists of the assertions currently being
processed, which are independent of the assertions seen so far. This
commit changes the index to be an ordinary integer and moves it to the
AssertionPipeline. Additionally, it abstracts access to the index in
preparation for splitting AssertionPipeline into three vectors (see
issue #2473).

src/preprocessing/assertion_pipeline.cpp
src/preprocessing/assertion_pipeline.h
src/preprocessing/passes/apply_substs.cpp
src/preprocessing/passes/non_clausal_simp.cpp
src/preprocessing/preprocessing_pass_context.cpp
src/preprocessing/preprocessing_pass_context.h
src/smt/smt_engine.cpp

index 382b1eb63cfa6b8081a2711f7ec759d750bbc94a..7408f4ba3a4b686dbe8fa11ba2a36ffa0e30d855 100644 (file)
 #include "expr/node_manager.h"
 #include "proof/proof.h"
 #include "proof/proof_manager.h"
+#include "theory/rewriter.h"
 
 namespace CVC4 {
 namespace preprocessing {
 
 AssertionPipeline::AssertionPipeline()
-    : d_realAssertionsEnd(0), d_assumptionsStart(0), d_numAssumptions(0)
+    : d_realAssertionsEnd(0),
+      d_storeSubstsInAsserts(false),
+      d_substsIndex(0),
+      d_assumptionsStart(0),
+      d_numAssumptions(0)
 {
 }
 
@@ -84,5 +89,27 @@ void AssertionPipeline::replace(size_t i, const std::vector<Node>& ns)
   }
 }
 
+void AssertionPipeline::enableStoreSubstsInAsserts()
+{
+  d_storeSubstsInAsserts = true;
+  d_substsIndex = d_nodes.size();
+  d_nodes.push_back(NodeManager::currentNM()->mkConst<bool>(true));
+}
+
+void AssertionPipeline::disableStoreSubstsInAsserts()
+{
+  d_storeSubstsInAsserts = false;
+}
+
+void AssertionPipeline::addSubstitutionNode(Node n)
+{
+  Assert(d_storeSubstsInAsserts);
+  Assert(n.getKind() == kind::EQUAL);
+  d_nodes[d_substsIndex] = theory::Rewriter::rewrite(
+      NodeManager::currentNM()->mkNode(kind::AND, n, d_nodes[d_substsIndex]));
+  Assert(theory::Rewriter::rewrite(d_nodes[d_substsIndex])
+         == d_nodes[d_substsIndex]);
+}
+
 }  // namespace preprocessing
 }  // namespace CVC4
index cc9d1c2af2db77aef284eda7c196165f5698e593..b133bc490bba327ce79549f85133d163b12eae6e 100644 (file)
@@ -95,6 +95,37 @@ class AssertionPipeline
 
   void updateRealAssertionsEnd() { d_realAssertionsEnd = d_nodes.size(); }
 
+  /**
+   * Returns true if substitutions must be stored as assertions. This is for
+   * example the case when we do incremental solving.
+   */
+  bool storeSubstsInAsserts() { return d_storeSubstsInAsserts; }
+
+  /**
+   * Enables storing substitutions as assertions.
+   */
+  void enableStoreSubstsInAsserts();
+
+  /**
+   * Disables storing substitutions as assertions.
+   */
+  void disableStoreSubstsInAsserts();
+
+  /**
+   * Adds a substitution node of the form (= lhs rhs) to the assertions.
+   */
+  void addSubstitutionNode(Node n);
+
+  /**
+   * Checks whether the assertion at a given index represents substitutions.
+   *
+   * @param i The index in question
+   */
+  bool isSubstsIndex(size_t i)
+  {
+    return d_storeSubstsInAsserts && i == d_substsIndex;
+  }
+
  private:
   /** The list of current assertions */
   std::vector<Node> d_nodes;
@@ -108,6 +139,20 @@ class AssertionPipeline
   /** Size of d_nodes when preprocessing starts */
   size_t d_realAssertionsEnd;
 
+  /**
+   * If true, we store the substitutions as assertions. This is necessary when
+   * doing incremental solving because we cannot apply them to existing
+   * assertions while preprocessing new assertions.
+   */
+  bool d_storeSubstsInAsserts;
+
+  /**
+   * The index of the assertions that holds the substitutions.
+   *
+   * TODO(#2473): replace by separate vector of substitution assertions.
+   */
+  size_t d_substsIndex;
+
   /** Index of the first assumption */
   size_t d_assumptionsStart;
   /** The number of assumptions */
index ddacc20c0381e9bdb835b431374e9ffd8d67bb60..791bb2dc705ce2c372a5ea25219e70bb7f95f544 100644 (file)
@@ -41,17 +41,12 @@ PreprocessingPassResult ApplySubsts::applyInternal(
     // TODO(#1255): Substitutions in incremental mode should be managed with a
     // proper data structure.
 
-    // When solving incrementally, all substitutions are piled into the
-    // assertion at d_substitutionsIndex: we don't want to apply substitutions
-    // to this assertion or information will be lost.
-    unsigned substs_index = d_preprocContext->getSubstitutionsIndex();
     theory::SubstitutionMap& substMap =
         d_preprocContext->getTopLevelSubstitutions();
     unsigned size = assertionsToPreprocess->size();
-    unsigned substitutionAssertion = substs_index > 0 ? substs_index : size;
     for (unsigned i = 0; i < size; ++i)
     {
-      if (i == substitutionAssertion)
+      if (assertionsToPreprocess->isSubstsIndex(i))
       {
         continue;
       }
index 4a0f3868958f7d7b86b0fcde1aa692c04aa2fa1b..139bf96a9dfa284bcb6360cfb0c9f2ef1afbc8a6 100644 (file)
@@ -76,13 +76,12 @@ PreprocessingPassResult NonClausalSimp::applyInternal(
 
   // Assert all the assertions to the propagator
   Trace("non-clausal-simplify") << "asserting to propagator" << std::endl;
-  unsigned substs_index = d_preprocContext->getSubstitutionsIndex();
   for (size_t i = 0, size = assertionsToPreprocess->size(); i < size; ++i)
   {
     Assert(Rewriter::rewrite((*assertionsToPreprocess)[i])
            == (*assertionsToPreprocess)[i]);
     // Don't reprocess substitutions
-    if (substs_index > 0 && i == substs_index)
+    if (assertionsToPreprocess->isSubstsIndex(i))
     {
       continue;
     }
@@ -344,14 +343,13 @@ PreprocessingPassResult NonClausalSimp::applyInternal(
   TheoryModel* m = d_preprocContext->getTheoryEngine()->getModel();
   Assert(m != nullptr);
   NodeManager* nm = NodeManager::currentNM();
-  NodeBuilder<> substitutionsBuilder(kind::AND);
   for (pos = newSubstitutions.begin(); pos != newSubstitutions.end(); ++pos)
   {
     Node lhs = (*pos).first;
     Node rhs = newSubstitutions.apply((*pos).second);
     // If using incremental, we must check whether this variable has occurred
     // before now. If it hasn't we can add this as a substitution.
-    if (substs_index == 0
+    if (!assertionsToPreprocess->storeSubstsInAsserts()
         || d_preprocContext->getSymsInAssertions().find(lhs)
                == d_preprocContext->getSymsInAssertions().end())
     {
@@ -366,16 +364,9 @@ PreprocessingPassResult NonClausalSimp::applyInternal(
       Trace("non-clausal-simplify")
           << "substitute: will notify SAT layer of substitution: " << eq
           << std::endl;
-      substitutionsBuilder << eq;
+      assertionsToPreprocess->addSubstitutionNode(eq);
     }
   }
-  // add to the last assertion if necessary
-  if (substitutionsBuilder.getNumChildren() > 0)
-  {
-    substitutionsBuilder << (*assertionsToPreprocess)[substs_index];
-    assertionsToPreprocess->replace(
-        substs_index, Rewriter::rewrite(Node(substitutionsBuilder)));
-  }
 
   NodeBuilder<> learnedBuilder(kind::AND);
   Assert(assertionsToPreprocess->getRealAssertionsEnd()
index 2d25502d133f5db8e0b168b07d12d83dc76a8d75..b04c05b9ee2233385f6ca3c77c9a81b5d5e90f71 100644 (file)
@@ -29,7 +29,6 @@ PreprocessingPassContext::PreprocessingPassContext(
     : d_smt(smt),
       d_resourceManager(resourceManager),
       d_iteRemover(iteRemover),
-      d_substitutionsIndex(smt->d_userContext, 0),
       d_topLevelSubstitutions(smt->d_userContext),
       d_circuitPropagator(circuitPropagator),
       d_symsInAssertions(smt->d_userContext)
index e37680538fd9504577718e904358a88a96ae92b1..37744151c6c33d86aa44b87bf03bace3a9b6a80b 100644 (file)
@@ -71,10 +71,6 @@ class PreprocessingPassContext
   /* Widen the logic to include the given theory. */
   void widenLogic(theory::TheoryId id);
 
-  unsigned getSubstitutionsIndex() const { return d_substitutionsIndex.get(); }
-
-  void setSubstitutionsIndex(unsigned i) { d_substitutionsIndex = i; }
-
   /** Gets a reference to the top-level substitution map */
   theory::SubstitutionMap& getTopLevelSubstitutions()
   {
@@ -101,9 +97,6 @@ class PreprocessingPassContext
   /** Instance of the ITE remover */
   RemoveTermFormulas* d_iteRemover;
 
-  /* Index for where to store substitutions */
-  context::CDO<unsigned> d_substitutionsIndex;
-
   /* The top level substitutions */
   theory::SubstitutionMap d_topLevelSubstitutions;
 
index 305c36d13ed6bb6f63af0c50e014d22d7503e26b..f451d12bd7c2178eb1e1947c23158aa1bd5daa3e 100644 (file)
@@ -3168,9 +3168,11 @@ void SmtEnginePrivate::processAssertions() {
     // TODO(b/1255): Substitutions in incremental mode should be managed with a
     // proper data structure.
 
-    // Placeholder for storing substitutions
-    d_preprocessingPassContext->setSubstitutionsIndex(d_assertions.size());
-    d_assertions.push_back(NodeManager::currentNM()->mkConst<bool>(true));
+    d_assertions.enableStoreSubstsInAsserts();
+  }
+  else
+  {
+    d_assertions.disableStoreSubstsInAsserts();
   }
 
   // Add dummy assertion in last position - to be used as a