Change old solver::propagate method

src/math/polysat/solver.cpp

@@ -453,22 +453,6 @@ namespace polysat {
         }
     }
 
-
-
-    // TODO: get rid of this or at least rename it
-    void solver::propagate(pvar v, rational const& val, signed_constraint c) {
-        // this looks weird... mixing propagation and conflict with c? also, the conflict should not be c but the whole of viable+c.
-        LOG("Propagation: " << assignment_pp(*this, v, val) << ", due to " << c);
-        if (m_viable.is_viable(v, val)) {
-            m_free_pvars.del_var_eh(v);
-            assign_core(v, val, justification::propagation(m_level));        
-        }
-        else {
-            UNREACHABLE();
-            // set_conflict(c);
-        }
-    }
-
     void solver::push_level() {
         ++m_level;
         m_trail.push_back(trail_instr_t::inc_level_i);
@@ -477,8 +461,6 @@ namespace polysat {
 #endif
     }
 
-
-
     void solver::pop_levels(unsigned num_levels) {
         if (num_levels == 0)
             return;
@@ -650,13 +632,33 @@ namespace polysat {
             return;
         case dd::find_t::singleton:
             // NOTE: this case may happen legitimately if all other possibilities were excluded by brute force search
+            // NOTE 2: probably not true anymore; viable::intersect should trigger all propagations now
+            DEBUG_CODE( UNREACHABLE(); );
             j = justification::propagation(m_level);
             break;
         case dd::find_t::multiple:
             j = justification::decision(m_level + 1);
             break;
         }
-        // Verify the value we're trying to assign
+        assign_verify(v, val, j);
+    }
+
+    void solver::assign_propagate(pvar v, rational const& val) {
+        LOG("Propagation: " << assignment_pp(*this, v, val));
+        SASSERT(!is_assigned(v));
+        SASSERT(m_viable.is_viable(v, val));
+        m_free_pvars.del_var_eh(v);
+        // NOTE: we do not have to check the univariate solver here.
+        //       Since we propagate, this means at most the single value 'val' is viable.
+        //       If it is not actually viable, the propagation loop will find out and enter the conflict state.
+        //       (However, if we do check here, we might find the conflict earlier. Might be worth a try.)
+        assign_core(v, val, justification::propagation(m_level));
+    }
+
+    /// Verify the value we're trying to assign against the univariate solver
+    void solver::assign_verify(pvar v, rational val, justification j) {
+        SASSERT(j.is_decision() || j.is_propagation());
+        // First, check evaluation of the currently-univariate constraints
         // TODO: we should add a better way to test constraints under assignments, without modifying the solver state.
         m_value[v] = val;
         m_search.push_assignment(v, val);
@@ -672,8 +674,7 @@ namespace polysat {
             case dd::find_t::singleton:
             case dd::find_t::multiple:
                 LOG("Fallback solver: " << assignment_pp(*this, v, val));
-                // NOTE: I don't think this can happen if viable::find_viable returned a singleton. since all values excluded by viable are true negatives.
+                SASSERT(!j.is_propagation());  // all excluded values are true negatives, so if j.is_propagation() the univariate solver must return unsat
-                SASSERT(!j.is_propagation());
                 j = justification::decision(m_level + 1);
                 break;
             case dd::find_t::empty:

src/math/polysat/solver.h

@@ -172,6 +172,8 @@ namespace polysat {
         void deactivate_constraint(signed_constraint c);
         unsigned level(sat::literal lit, clause const& cl);
 
+        void assign_propagate(pvar v, rational const& val);
+        void assign_verify(pvar v, rational val, justification j);
         void assign_core(pvar v, rational const& val, justification const& j);
         bool is_assigned(pvar v) const { return !m_justification[v].is_unassigned(); }
         bool is_decision(pvar v) const { return m_justification[v].is_decision(); }
@@ -181,7 +183,6 @@ namespace polysat {
         void propagate(sat::literal lit);
         void propagate(pvar v);
         bool propagate(pvar v, constraint* c);
-        void propagate(pvar v, rational const& val, signed_constraint c);
         bool propagate(sat::literal lit, clause& cl);
         void add_pwatch(constraint* c);
         void add_pwatch(constraint* c, pvar v);

src/math/polysat/viable.cpp

@@ -130,7 +130,7 @@ namespace polysat {
             rational val;
             switch (find_viable(v, val)) {
             case dd::find_t::singleton:
-                s.propagate(v, val, sc); // TBD why is sc used as justification? It should be all of viable
+                s.assign_propagate(v, val);
                 prop = true;
                 break;
             case dd::find_t::empty: