[proofs] [sat] Have SAT solver communicate whether it has optimized the assertion...
authorHaniel Barbosa <hanielbbarbosa@gmail.com>
Fri, 25 Mar 2022 21:25:04 +0000 (18:25 -0300)
committerGitHub <noreply@github.com>
Fri, 25 Mar 2022 21:25:04 +0000 (21:25 +0000)
This way both the SAT proof manager and the Proof Cnf Stream can properly track
proofs for clauses added at assertion levels below their original user level.

This commit, besides some minor tweaks, adds the hooks in the places where
a clause can be created with an optimized level and we need to track:

when the explanation of a propagation is requested
when a clause/lemma is added
when a lemma is added (from the backlog of lemmas added when Minisat was busy)
For clauses derived via resolution, this information is computed directly in the SAT proof manager.

This commit also reverts a change from #7790 where whether to optimize clauses was computed only once for the SAT solver. This cannot be the case because between different check-sat calls the state can change.

Finally, this commit enables proofs for several regressions that were previously incompatible with proofs. Since now proofs are compatible with optimizing the level of clauses during incremental solving, this commit should lead to performance improvements.

Fixes cvc5/cvc5-projects#314

src/prop/minisat/core/Solver.cc
src/prop/minisat/core/Solver.h
src/prop/proof_post_processor.cpp
src/prop/prop_engine.cpp
src/prop/sat_proof_manager.cpp
test/regress/cli/regress0/cores/issue3455.smt2
test/regress/cli/regress0/cores/issue4925.smt2
test/regress/cli/regress1/push-pop/fuzz_3_1.smt2
test/regress/cli/regress1/push-pop/fuzz_3_11.smt2
test/regress/cli/regress1/push-pop/fuzz_3_9.smt2

index 1294b9d330d5e499dd2069f39ca18a6f4a30dca5..64d77e97ad06c52a12c1d97d76c5c220692c165a 100644 (file)
@@ -142,9 +142,6 @@ Solver::Solver(Env& env,
       d_context(context),
       assertionLevel(0),
       d_pfManager(nullptr),
-      d_assertionLevelOnly(
-          (options().smt.produceProofs || options().smt.unsatCores)
-          && options().base.incrementalSolving),
       d_enable_incremental(enableIncremental),
       minisat_busy(false)
       // Parameters (user settable):
@@ -256,7 +253,8 @@ Var Solver::newVar(bool sign, bool dvar, bool isTheoryAtom, bool preRegister, bo
 
     setDecisionVar(v, dvar);
 
-    Trace("minisat") << "new var " << v << std::endl;
+    Trace("minisat") << "new var " << v << " with assertion level "
+                     << assertionLevel << std::endl;
 
     // If the variable is introduced at non-zero level, we need to reintroduce it on backtracks
     if (preRegister)
@@ -342,15 +340,15 @@ CRef Solver::reason(Var x) {
 
   // Compute the assertion level for this clause
   int explLevel = 0;
-  if (d_assertionLevelOnly)
+  if (assertionLevelOnly())
   {
     explLevel = assertionLevel;
   }
   else
   {
-    int i, j;
+    int i, j, size;
     Lit prev = lit_Undef;
-    for (i = 0, j = 0; i < explanation.size(); ++i)
+    for (i = 0, j = 0, size = explanation.size(); i < size; ++i)
     {
       // This clause is valid theory propagation, so its level is the level of
       // the top literal
@@ -383,13 +381,6 @@ CRef Solver::reason(Var x) {
     }
     explanation.shrink(i - j);
 
-    Trace("pf::sat") << "Solver::reason: explanation = ";
-    for (int k = 0; k < explanation.size(); ++k)
-    {
-      Trace("pf::sat") << explanation[k] << " ";
-    }
-    Trace("pf::sat") << std::endl;
-
     // We need an explanation clause so we add a fake literal
     if (j == 1)
     {
@@ -398,6 +389,14 @@ CRef Solver::reason(Var x) {
     }
   }
 
+  Trace("pf::sat") << "..adding in lvl " << explLevel
+                   << " (assertion lvl: " << assertionLevel << ")\n";
+  if (needProof() && explLevel < assertionLevel)
+  {
+    Trace("pf::sat") << "..user level is " << userContext()->getLevel() << "\n";
+    Assert(userContext()->getLevel() == (assertionLevel + 1));
+    d_proxy->notifyCurrPropagationInsertedAtLevel(explLevel);
+  }
   // Construct the reason
   CRef real_reason = ca.alloc(explLevel, explanation, true);
   vardata[x] = VarData(
@@ -417,13 +416,13 @@ bool Solver::addClause_(vec<Lit>& ps, bool removable, ClauseId& id)
     Lit p; int i, j;
 
     // Which user-level to assert this clause at
-    int clauseLevel = (removable && !d_assertionLevelOnly) ? 0 : assertionLevel;
+    int clauseLevel = (removable && !assertionLevelOnly()) ? 0 : assertionLevel;
 
     // Check the clause for tautologies and similar
     int falseLiteralsCount = 0;
     for (i = j = 0, p = lit_Undef; i < ps.size(); i++) {
       // Update the level
-      clauseLevel = d_assertionLevelOnly
+      clauseLevel = assertionLevelOnly()
                         ? assertionLevel
                         : std::max(clauseLevel, intro_level(var(ps[i])));
       // Tautologies are ignored
@@ -465,12 +464,15 @@ bool Solver::addClause_(vec<Lit>& ps, bool removable, ClauseId& id)
     // If we are in solve_ or propagate
     if (minisat_busy)
     {
-      Trace("pf::sat") << "Add clause adding a new lemma: ";
-      for (int k = 0; k < ps.size(); ++k) {
-        Trace("pf::sat") << ps[k] << " ";
+      if (TraceIsOn("pf::sat"))
+      {
+        Trace("pf::sat") << "Add clause adding a new lemma: ";
+        for (int k = 0, size = ps.size(); k < size; ++k)
+        {
+          Trace("pf::sat") << ps[k] << " ";
+        }
+        Trace("pf::sat") << std::endl;
       }
-      Trace("pf::sat") << std::endl;
-
       lemmas.push();
       ps.copyTo(lemmas.last());
       lemmas_removable.push(removable);
@@ -509,7 +511,22 @@ bool Solver::addClause_(vec<Lit>& ps, bool removable, ClauseId& id)
         cr = ca.alloc(clauseLevel, ps, false);
         clauses_persistent.push(cr);
         attachClause(cr);
-
+        if (needProof() && clauseLevel < assertionLevel)
+        {
+          if (TraceIsOn("pf::sat"))
+          {
+            Trace("pf::sat") << "addClause_: ";
+            for (int k = 0, size = ps.size(); k < size; ++k)
+            {
+              Trace("pf::sat") << ps[k] << " ";
+            }
+            Trace("pf::sat") << " clause/assert levels " << clauseLevel << " / "
+                             << assertionLevel << "\n";
+          }
+          SatClause satClause;
+          MinisatSatSolver::toSatClause(ca[cr], satClause);
+          d_proxy->notifyClauseInsertedAtLevel(satClause, clauseLevel);
+        }
         if (options().smt.unsatCores || needProof())
         {
           if (ps.size() == falseLiteralsCount)
@@ -524,45 +541,48 @@ bool Solver::addClause_(vec<Lit>& ps, bool removable, ClauseId& id)
       }
 
       // Check if it propagates
-      if (ps.size() == falseLiteralsCount + 1) {
-        if(assigns[var(ps[0])] == l_Undef) {
-          Assert(assigns[var(ps[0])] != l_False);
-          uncheckedEnqueue(ps[0], cr);
-          Trace("cores") << "i'm registering a unit clause, maybe input"
+      if (ps.size() == falseLiteralsCount + 1 && assigns[var(ps[0])] == l_Undef)
+      {
+        Assert(assigns[var(ps[0])] != l_False);
+        uncheckedEnqueue(ps[0], cr);
+        Trace("pf::sat") << "Registering a unit clause " << ps[0]
+                         << ", maybe input, with assigned var value "
+                         << (assigns[var(ps[0])] == l_True
+                                 ? "true"
+                                 : (assigns[var(ps[0])] == l_False ? "false"
+                                                                   : "undef"))
+                         << ", user_level(" << user_level(var(ps[0])) << ")"
                          << std::endl;
-          if (ps.size() == 1)
+        if (ps.size() == 1)
+        {
+          // We need to do this so that the closedness check, if being done,
+          // goes through when we have unit assumptions whose literal has
+          // already been registered, as the ProofCnfStream will not register
+          // them and as they are not the result of propagation will be left
+          // hanging in assumptions accumulator
+          if (needProof())
           {
-            // We need to do this so that the closedness check, if being done,
-            // goes through when we have unit assumptions whose literal has
-            // already been registered, as the ProofCnfStream will not register
-            // them and as they are not the result of propagation will be left
-            // hanging in assumptions accumulator
-            if (needProof())
+            d_pfManager->registerSatLitAssumption(ps[0]);
+          }
+        }
+        CRef confl = propagate(CHECK_WITHOUT_THEORY);
+        if (!(ok = (confl == CRef_Undef)))
+        {
+          if (needProof())
+          {
+            if (ca[confl].size() == 1)
             {
-              d_pfManager->registerSatLitAssumption(ps[0]);
+              d_pfManager->finalizeProof(ca[confl][0]);
             }
-          }
-          CRef confl = propagate(CHECK_WITHOUT_THEORY);
-          if(! (ok = (confl == CRef_Undef)) ) {
-            if (needProof())
+            else
             {
-              if (ca[confl].size() == 1)
-              {
-                d_pfManager->finalizeProof(ca[confl][0]);
-              }
-              else
-              {
-                d_pfManager->finalizeProof(ca[confl]);
-              }
+              d_pfManager->finalizeProof(ca[confl]);
             }
           }
-          return ok;
-        } else {
-          return ok;
         }
+        return ok;
       }
     }
-
     return true;
 }
 
@@ -1140,6 +1160,7 @@ void Solver::uncheckedEnqueue(Lit p, CRef from)
     Trace("minisat") << "unchecked enqueue of " << p << " ("
                      << trail_index(var(p)) << ") trail size is "
                      << trail.size() << " cap is " << trail.capacity()
+                     << ", assertion level is " << assertionLevel
                      << ", reason is " << from << ", ";
     if (from == CRef_Lazy)
     {
@@ -1443,7 +1464,11 @@ void Solver::removeClausesAboveLevel(vec<CRef>& cs, int level)
     for (i = j = 0; i < cs.size(); i++){
         Clause& c = ca[cs[i]];
         if (c.level() > level) {
-          Assert(!locked(c));
+          SatClause satClause;
+          vec<Lit> clauseLits;
+          MinisatSatSolver::toSatClause(c, satClause);
+          MinisatSatSolver::toMinisatClause(satClause, clauseLits);
+          Assert(!locked(c)) << "Locked " << clauseLits;
           removeClause(cs[i]);
         } else {
             cs[j++] = cs[i];
@@ -1559,7 +1584,7 @@ lbool Solver::search(int nof_conflicts)
       }
       else
       {
-        CRef cr = ca.alloc(d_assertionLevelOnly ? assertionLevel : max_level,
+        CRef cr = ca.alloc(assertionLevelOnly() ? assertionLevel : max_level,
                            learnt_clause,
                            true);
         clauses_removable.push(cr);
@@ -1569,6 +1594,12 @@ lbool Solver::search(int nof_conflicts)
         if (needProof())
         {
           d_pfManager->endResChain(ca[cr]);
+          if (TraceIsOn("pf::sat") && ca[cr].level() < assertionLevel)
+          {
+            Trace("pf::sat")
+                << "learnt_clause: " << ca[cr] << " clause/assert levels "
+                << ca[cr].level() << " / " << assertionLevel << "\n";
+          }
         }
       }
 
@@ -1961,7 +1992,12 @@ void Solver::pop()
   Assert(d_enable_incremental);
 
   Assert(decisionLevel() == 0);
-
+  // Notify sat proof manager that we have popped and now potentially we need to
+  // retrieve the proofs for the clauses inserted into optimized levels
+  if (needProof())
+  {
+    d_pfManager->notifyPop();
+  }
   // Pop the trail below the user level
   --assertionLevel;
   Trace("minisat") << "in user pop, decreasing assertion level to "
@@ -2078,7 +2114,7 @@ CRef Solver::updateLemmas() {
     if (lemma.size() > 1) {
       // If the lemmas is removable, we can compute its level by the level
       int clauseLevel = assertionLevel;
-      if (removable && !d_assertionLevelOnly)
+      if (removable && !assertionLevelOnly())
       {
         clauseLevel = 0;
         for (int k = 0; k < lemma.size(); ++k)
@@ -2088,6 +2124,24 @@ CRef Solver::updateLemmas() {
       }
 
       lemma_ref = ca.alloc(clauseLevel, lemma, removable);
+      // notify cnf stream that this clause's proof must be saved to resist
+      // context-popping
+      if (needProof() && clauseLevel < assertionLevel)
+      {
+        if (TraceIsOn("pf::sat"))
+        {
+          Trace("pf::sat") << "updateLemmas: ";
+          for (int k = 0, size = lemma.size(); k < size; ++k)
+          {
+            Trace("pf::sat") << lemma[k] << " ";
+          }
+          Trace("pf::sat") << " clause/assert levels " << clauseLevel << " / "
+                           << assertionLevel << "\n";
+        }
+        SatClause satClause;
+        MinisatSatSolver::toSatClause(ca[lemma_ref], satClause);
+        d_proxy->notifyClauseInsertedAtLevel(satClause, clauseLevel);
+      }
       if (removable) {
         clauses_removable.push(lemma_ref);
       } else {
@@ -2184,5 +2238,11 @@ bool Solver::needProof() const
          && options().smt.proofMode != options::ProofMode::PP_ONLY;
 }
 
+bool Solver::assertionLevelOnly() const
+{
+  return options().smt.unsatCores && !needProof()
+         && options().base.incrementalSolving;
+}
+
 }  // namespace Minisat
 }  // namespace cvc5
index 2f19c6e1620cd320fa05e5ac3502ec501b4ee1d3..2f027788ea222cc9c23b3eaff8606d23e524cda5 100644 (file)
@@ -74,7 +74,7 @@ class Solver : protected EnvObj
   /** The pointer to the proxy that provides interfaces to the SMT engine */
   cvc5::prop::TheoryProxy* d_proxy;
 
-  /** The context from the SMT solver */
+  /** The contexts from the SMT solver */
   cvc5::context::Context* d_context;
 
   /** The current assertion level (user) */
@@ -94,17 +94,6 @@ class Solver : protected EnvObj
   int getAssertionLevel() const { return assertionLevel; }
 
  protected:
-  /*
-   * Returns true if the solver should add all clauses at the current assertion
-   * level.
-   *
-   * FIXME: This is a workaround. Currently, our resolution proofs do not
-   * handle clauses with a lower-than-assertion-level correctly because the
-   * resolution proofs get removed when popping the context but the SAT solver
-   * keeps using them.
-   */
-  const bool d_assertionLevelOnly;
-
   /** Do we allow incremental solving */
   bool d_enable_incremental;
 
@@ -176,6 +165,16 @@ public:
   */
  bool needProof() const;
 
+ /*
+  * Returns true if the solver should add all clauses at the current assertion
+  * level.
+  *
+  * FIXME (cvc5-projects/issues/503): This is a workaround. While proofs are now
+  * compatible with the assertion level optimization, it has to be seen for
+  * non-sat-proofs-based unsat cores.
+  */
+ bool assertionLevelOnly() const;
+
  // Less than for literals in a lemma
  struct lemma_lt
  {
index b852654279e0ff3bfce8ada1b9bc4882876ace81..890ad662c644f0d81f4b40c37276e26bfa5b15bf 100644 (file)
@@ -34,6 +34,11 @@ bool ProofPostprocessCallback::shouldUpdate(std::shared_ptr<ProofNode> pn,
 {
   bool result = pn->getRule() == PfRule::ASSUME
                 && d_proofCnfStream->hasProofFor(pn->getResult());
+  if (TraceIsOn("prop-proof-pp") && !result && pn->getRule() == PfRule::ASSUME)
+  {
+    Trace("prop-proof-pp") << "- Ignoring no-proof assumption "
+                           << pn->getResult() << "\n";
+  }
   // check if should continue traversing
   if (d_proofCnfStream->isBlocked(pn))
   {
@@ -50,8 +55,9 @@ bool ProofPostprocessCallback::update(Node res,
                                       CDProof* cdp,
                                       bool& continueUpdate)
 {
-  Trace("prop-proof-pp-debug")
-      << "- Post process " << id << " " << children << " / " << args << "\n";
+  Trace("prop-proof-pp") << "- Post process " << id << " " << res << " : "
+                         << children << " / " << args << "\n"
+                         << push;
   Assert(id == PfRule::ASSUME);
   // we cache based on the assumption node, not the proof node, since there
   // may be multiple occurrences of the same node.
@@ -61,7 +67,7 @@ bool ProofPostprocessCallback::update(Node res,
       d_assumpToProof.find(f);
   if (it != d_assumpToProof.end())
   {
-    Trace("prop-proof-pp-debug") << "...already computed" << std::endl;
+    Trace("prop-proof-pp") << "...already computed" << std::endl;
     pfn = it->second;
   }
   else
@@ -77,6 +83,7 @@ bool ProofPostprocessCallback::update(Node res,
     }
     d_assumpToProof[f] = pfn;
   }
+  Trace("prop-proof-pp") << pop;
   // connect the proof
   cdp->addProof(pfn);
   // do not recursively process the result
index 678ef2107806868235e1ea5704458e7546312b08..12116344c6770ed1ba49dbd57e3a88204bc064f8 100644 (file)
@@ -652,6 +652,9 @@ std::shared_ptr<ProofNode> PropEngine::getProof()
   {
     return nullptr;
   }
+  Trace("sat-proof") << "PropEngine::getProof: getting proof with cnfStream's "
+                        "lazycdproof cxt lvl "
+                     << userContext()->getLevel() << "\n";
   return d_ppm->getProof();
 }
 
index d31b1273a7a6f98b6c4878652bbbda235b627b74..63967a23fe53845db61f81cc715ae1a4503cb936 100644 (file)
@@ -149,9 +149,9 @@ void SatProofManager::endResChain(const Minisat::Clause& clause)
   }
   Node conclusion = getClauseNode(clause);
   int clauseLevel = clause.level() + 1;
-  if (clauseLevel < userContext()->getLevel())
+  if (clauseLevel < userContext()->getLevel()
+      && !d_resChains.hasGenerator(conclusion))
   {
-    Assert(!d_optResLevels.count(conclusion));
     d_optResLevels[conclusion] = clauseLevel;
     Trace("sat-proof") << "SatProofManager::endResChain: ..clause's lvl "
                        << clause.level() + 1 << " below curr user level "
index 7050e8c7dd58120981e00077b86a13dc69d80d6f..b922bb7fa68dae5371a2302410b925ce9a588557 100644 (file)
@@ -1,4 +1,4 @@
-; COMMAND-LINE: -q --incremental --no-check-proofs
+; COMMAND-LINE: -q --incremental
 ; EXPECT: sat
 ; EXPECT: sat
 ; EXPECT: unsat
index 62428b5651c0a6214425bb312799272e54a38671..f1002ed75df89cc648c233ca81e179955daa4efb 100644 (file)
@@ -1,4 +1,3 @@
-; COMMAND-LINE: --no-check-proofs
 ; EXPECT: sat
 ; EXPECT: unsat
 ; EXPECT: unsat
index 1f3488b16f2ce1ad303a06c37e04fc1cd15fa128..bf2d2a8c312413f8cb107f0a468a3c898a697da2 100644 (file)
@@ -1,4 +1,4 @@
-; COMMAND-LINE: --incremental --no-produce-proofs
+; COMMAND-LINE: --incremental
 ; EXPECT: sat
 ; EXPECT: sat
 ; EXPECT: unsat
index 4e3ebb62516e9dd67cbe74ffaa453d0bd98ac84d..81fe3b046d6ff0fe6f0d1b092afc6d96d73b251b 100644 (file)
@@ -1,4 +1,4 @@
-; COMMAND-LINE: --incremental --no-produce-proofs
+; COMMAND-LINE: --incremental
 ; EXPECT: sat
 ; EXPECT: sat
 ; EXPECT: sat
index f7d2ae60e0a0d2a9aad760c8999d343c9ef358f5..96aaf9f514e7b0ad6e10d96e220cac7dc0b3a575 100644 (file)
@@ -1,4 +1,4 @@
-; COMMAND-LINE: --incremental --no-produce-proofs
+; COMMAND-LINE: --incremental
 ; EXPECT: sat
 ; EXPECT: sat
 ; EXPECT: sat