[proofs] [sat] Make SAT assumption bookeeping robust to incremental (#8536)
authorHaniel Barbosa <hanielbbarbosa@gmail.com>
Mon, 4 Apr 2022 14:12:39 +0000 (11:12 -0300)
committerGitHub <noreply@github.com>
Mon, 4 Apr 2022 14:12:39 +0000 (14:12 +0000)
Similarly to what happened with proofs of optimized SAT clauses getting lost (in the sense of inserted at lower assertion levels than the current one, during incremental solving), we need to make the bookeeping of the current SAT assumptions in the SAT proof manager robust to context popping. Not doing so can break the final proof construction, which relies on this information. A regression is added which showed the issue via the openness of a step added to the lazy proof chain (even though this did not lead to issues in the final proof).

This commit extends the OptimizedClausesManager to also optionally track a hash set of nodes to have nodes added to it during popping. This is hooked in the SAT proof manager to the SAT assumptions hash set.

There are four instances of notifications to the SAT proof manager of SAT assumptions: two in the proof CNF stream and two in the SAT solver. We only need to worry about the proof CNF stream ones, since the SAT solver ones are for literals (which do
not have assertion levels) and for some cases of the final conflict clause generated, which is always at the current assertion level. For the proof CNF stream ones it suffices to notify the SAT proof manager when the CNF stream itself is notified that a propagation or clause was inserted at an optimized level, as these are the only possible cases.

src/prop/opt_clauses_manager.cpp
src/prop/opt_clauses_manager.h
src/prop/proof_cnf_stream.cpp
src/prop/sat_proof_manager.cpp
src/prop/sat_proof_manager.h
test/regress/cli/CMakeLists.txt
test/regress/cli/regress0/proofs/tricky-sat-assumption-incremental-bookeeping.smt2 [new file with mode: 0644]

index e3ae842b5ad2eadf8a8a3110530b1ee2eccc8d5e..6f716bcc551acfa6b7051286d766917a12078f9c 100644 (file)
@@ -27,7 +27,9 @@ OptimizedClausesManager::OptimizedClausesManager(
     : context::ContextNotifyObj(context),
       d_context(context),
       d_optProofs(optProofs),
-      d_parentProof(parentProof)
+      d_parentProof(parentProof),
+      d_nodeHashSet(nullptr),
+      d_nodeLevels(nullptr)
 {
 }
 
@@ -78,8 +80,42 @@ void OptimizedClausesManager::contextNotifyPop()
     }
     it = d_optProofs.erase(it);
   }
+  if (d_nodeHashSet)
+  {
+    Assert(d_nodeLevels);
+    // traverse mapping from context levels to nodes so that we can reinsert the
+    // nodes that are below the current level being popped. The entries in the
+    // map at or above this level are deleted.
+    for (auto it = d_nodeLevels->cbegin(); it != d_nodeLevels->cend();)
+    {
+      if (it->first <= newLvl)
+      {
+        Trace("sat-proof") << "Should re-add SAT assumptions of [" << it->first
+                           << "]:\n";
+        for (const auto& assumption : it->second)
+        {
+          Trace("sat-proof") << "\t- " << assumption << "\n";
+          // Note that since it's a hash set we do not care about repetitions
+          d_nodeHashSet->insert(assumption);
+        }
+        ++it;
+        continue;
+      }
+      Trace("sat-proof") << "Should remove from map assumptions of ["
+                         << it->first << "]: " << it->second << "\n";
+      it = d_nodeLevels->erase(it);
+    }
+  }
   Trace("sat-proof") << pop;
 }
 
+void OptimizedClausesManager::trackNodeHashSet(
+    context::CDHashSet<Node>* nodeHashSet,
+    std::map<int, std::vector<Node>>* nodeLevels)
+{
+  d_nodeHashSet = nodeHashSet;
+  d_nodeLevels = nodeLevels;
+}
+
 }  // namespace prop
 }  // namespace cvc5::internal
index a629f1a3b316906d3abbc9d6a250c71267ce8612..505ad7480cc82bb123edfc6b44b5cccac8207b30 100644 (file)
@@ -19,6 +19,7 @@
 #define CVC5__PROP__OPT_CLAUSES_MANAGER_H
 
 #include "context/cdhashmap.h"
+#include "context/cdhashset.h"
 #include "expr/node.h"
 #include "proof/proof.h"
 
@@ -51,6 +52,21 @@ class OptimizedClausesManager : context::ContextNotifyObj
       CDProof* parentProof,
       std::map<int, std::vector<std::shared_ptr<ProofNode>>>& optProofs);
 
+  /** Adds a hash set of nodes to be tracked and updated when popping
+   *
+   * This method can be used when it is necessary to track, in a
+   * context-dependent manner, other information, in a node hash set, beyond the
+   * proofs associated with given clauses. For example, the SAT proof manager
+   * needs to bookeep the current assumptions of the SAT solver, which are
+   * stored in a node hash set.
+   *
+   * @param nodeHashSet the node hash set to be updated when context pops
+   * @param nodeLevels a mapping from context levels to nodes to be reinserted
+   * at these levels
+   */
+  void trackNodeHashSet(context::CDHashSet<Node>* nodeHashSet,
+                        std::map<int, std::vector<Node>>* nodeLevels);
+
  private:
   /** Event triggered by the tracked contexting popping
    *
@@ -66,6 +82,10 @@ class OptimizedClausesManager : context::ContextNotifyObj
   std::map<int, std::vector<std::shared_ptr<ProofNode>>>& d_optProofs;
   /** Proof to be updated when context pops. */
   CDProof* d_parentProof;
+  /** Node hash set to be updated when context pops, if such a set is tracked */
+  context::CDHashSet<Node>* d_nodeHashSet;
+  /** Map from levels to proof nodes. */
+  std::map<int, std::vector<Node>>* d_nodeLevels;
 };
 
 }  // namespace prop
index 511f3db7972406a61da1d54d59f978b085b2d7bc..ac70a0cd4ca2491246f96f2a61cfc66293a78a67 100644 (file)
@@ -636,7 +636,11 @@ void ProofCnfStream::notifyCurrPropagationInsertedAtLevel(int explLevel)
   Trace("cnf-debug") << "\t..saved pf {" << currPropagationProcPf << "} "
                      << *currPropagationProcPf.get() << "\n";
   d_optClausesPfs[explLevel + 1].push_back(currPropagationProcPf);
-
+  // Notify SAT proof manager that the propagation (which is a SAT assumption)
+  // had its level optimized
+  d_satPM->notifyAssumptionInsertedAtLevel(explLevel,
+                                           d_currPropagationProccessed);
+  // Reset
   d_currPropagationProccessed = Node::null();
 }
 
@@ -654,6 +658,9 @@ void ProofCnfStream::notifyClauseInsertedAtLevel(const SatClause& clause,
       d_env.getProofNodeManager()->clone(d_proof.getProofFor(clauseNode));
   Assert(clauseCnfPf->getRule() != PfRule::ASSUME);
   d_optClausesPfs[clLevel + 1].push_back(clauseCnfPf);
+  // Notify SAT proof manager that the propagation (which is a SAT assumption)
+  // had its level optimized
+  d_satPM->notifyAssumptionInsertedAtLevel(clLevel, clauseNode);
 }
 
 Node ProofCnfStream::getClauseNode(const SatClause& clause)
index 61d8719df9d2e7760027bb28b8587c253dcce82e..d2704ae738bf238609198942026fa42d6e258066 100644 (file)
@@ -39,6 +39,7 @@ SatProofManager::SatProofManager(Env& env,
 {
   d_true = NodeManager::currentNM()->mkConst(true);
   d_false = NodeManager::currentNM()->mkConst(false);
+  d_optResManager.trackNodeHashSet(&d_assumptions, &d_assumptionLevels);
 }
 
 void SatProofManager::printClause(const Minisat::Clause& clause)
@@ -796,6 +797,13 @@ void SatProofManager::registerSatAssumptions(const std::vector<Node>& assumps)
   }
 }
 
+void SatProofManager::notifyAssumptionInsertedAtLevel(int level,
+                                                      Node assumption)
+{
+  Assert(d_assumptions.contains(assumption));
+  d_assumptionLevels[level].push_back(assumption);
+}
+
 void SatProofManager::notifyPop()
 {
   for (context::CDHashMap<Node, int>::const_iterator it =
@@ -804,7 +812,7 @@ void SatProofManager::notifyPop()
        ++it)
   {
     // Save into map the proof of the resolution chain. We copy to prevent the
-    // proof node saved to be restored to suffering unintended updates. This is
+    // proof node saved to be restored of suffering unintended updates. This is
     // *necessary*.
     std::shared_ptr<ProofNode> clauseResPf =
         d_env.getProofNodeManager()->clone(d_resChains.getProofFor(it->first));
index e68db690dc2205fb38d6bf78e7fa0500bc31dbaf..8058daccbb45d623117c6e739fedff9e5fb8cc4c 100644 (file)
@@ -364,6 +364,10 @@ class SatProofManager : protected EnvObj
   /** Notify this proof manager that the SAT solver has user-context popped. */
   void notifyPop();
 
+  /** Notify this proof manager that a SAT assumption has had its level
+   * optmized. */
+  void notifyAssumptionInsertedAtLevel(int level, Node assumption);
+
  private:
   /** Ends resolution chain concluding clause
    *
@@ -385,7 +389,8 @@ class SatProofManager : protected EnvObj
    * - <(or ~l6 l7), l6>
    * - <(or l4 ~l7), l7>
    *
-   * The resulting children and arguments for the CHAIN_RESOLUTION proof step would be:
+   * The resulting children and arguments for the CHAIN_RESOLUTION proof step
+   * would be:
    * - [(or l3 l5 l6 l7), ~l5, (or ~l6 l7), (or l4 ~l7)]
    * - [l5, l6, l7]
    * and the proof step
@@ -598,6 +603,12 @@ class SatProofManager : protected EnvObj
    * manager when the context pops.
    */
   std::map<int, std::vector<std::shared_ptr<ProofNode>>> d_optResProofs;
+  /** Maps assertion level to assumptions
+   *
+   * As above, used by d_optResManager to update the assumption set as the
+   * context pops, so that we track the correct current SAT assumptions.
+   */
+  std::map<int, std::vector<Node>> d_assumptionLevels;
   /** Manager for optimized resolution conclusions inserted at assertion levels
    * below the current user level. */
   OptimizedClausesManager d_optResManager;
index e40ee670e9f450c6aac436db8ed71a0c2745ecf4..71d41259679d386938c584db2cece2d2e7049ebc 100644 (file)
@@ -973,6 +973,7 @@ set(regress_0_tests
   regress0/proofs/qgu-fuzz-4-bool-chainres-postprocessing-singleton.smt2
   regress0/proofs/qgu-fuzz-5-bool-open-sat.smt2
   regress0/proofs/scope.smt2
+  regress0/proofs/tricky-sat-assumption-incremental-bookeeping.smt2
   regress0/proofs/trust-subs-eq-open.smt2
   regress0/push-pop/boolean/fuzz_12.smt2
   regress0/push-pop/boolean/fuzz_13.smt2
diff --git a/test/regress/cli/regress0/proofs/tricky-sat-assumption-incremental-bookeeping.smt2 b/test/regress/cli/regress0/proofs/tricky-sat-assumption-incremental-bookeeping.smt2
new file mode 100644 (file)
index 0000000..2884887
--- /dev/null
@@ -0,0 +1,17 @@
+; COMMAND-LINE: -i --produce-proofs --proof-check=eager
+; EXPECT: sat
+; EXPECT: unsat
+(set-logic QF_UFLIA)
+(declare-fun x (Int) Bool)
+(declare-fun y (Int) Bool)
+(declare-fun z (Int) Int)
+(declare-fun _z (Int) Int)
+(assert (= (x 0) (not (y 0))))
+(assert (= (y 0) (> 0 (z 0))))
+(assert (= (z 0) (_z 0)))
+(assert (= 0 (_z 0)))
+(push)
+(check-sat)
+(pop)
+(assert (not (x 0)))
+(check-sat)