Logo AND Algorithmique Numérique Distribuée

Public GIT Repository
Fix bug with immediate conflict detection
authorMaxwell Pirtle <maxwellpirtle@gmail.com>
Wed, 5 Apr 2023 12:58:29 +0000 (14:58 +0200)
committerMaxwell Pirtle <maxwellpirtle@gmail.com>
Thu, 6 Apr 2023 08:55:29 +0000 (10:55 +0200)
Detecting immediate conflicts for an event is
important as part of the clean up phase of
UDPOR. There was a typo that gave incorrect
results for the actual conflicts that were
detected which was resolved.

NOTE: There is still a problem with event
deletion. The issue is that previous configurations
will contain references to events which have been
deleted in the extension set which is computed
for them (ex(C)). I'm not quite sure how to
handle this at the moment...

src/mc/explo/UdporChecker.cpp
src/mc/explo/udpor/EventSet.cpp
src/mc/explo/udpor/EventSet.hpp
src/mc/explo/udpor/Unfolding.cpp
src/mc/explo/udpor/Unfolding.hpp
src/mc/explo/udpor/UnfoldingEvent.cpp

index 42a8f54..4de2bdb 100644 (file)
@@ -250,55 +250,70 @@ UnfoldingEvent* UdporChecker::select_next_unfolding_event(const EventSet& A, con
 
 void UdporChecker::clean_up_explore(const UnfoldingEvent* e, const Configuration& C, const EventSet& D)
 {
-  // // The "clean-up set" conceptually represents
-  // // those events which will no longer be considered
-  // // by UDPOR during its exploration. The concept is
-  // // introduced to avoid modification during iteration
-  // // over the current unfolding to determine who needs to
-  // // be removed. Since sets are unordered, it's quite possible
-  // // that e.g. two events `e` and `e'` such that `e < e'`
-  // // which are determined eligible for removal are removed
-  // // in the order `e` and then `e'`. Determining that `e'`
-  // // needs to be removed requires that its history be in
-  // // tact to e.g. compute the conflicts with the event.
-  // //
-  // // Thus, we compute the set and remove all of the events
-  // // at once in lieu of removing events while iterating over them.
-  // // We can hypothesize that processing the events in reverse
-  // // topological order would prevent any issues concerning
-  // // the order in which are processed
-  // EventSet clean_up_set;
-
-  // // Q_(C, D, U) = C u D u U (complicated expression)
-  // // See page 9 of "Unfolding-based Partial Order Reduction"
-
-  // // "C u D" portion
-  // const EventSet C_union_D = C.get_events().make_union(D);
-
-  // // "U (complicated expression)" portion
-  // const EventSet conflict_union = std::accumulate(
-  //     C_union_D.begin(), C_union_D.end(), EventSet(), [&](const EventSet acc, const UnfoldingEvent* e_prime) {
-  //       return acc.make_union(unfolding.get_immediate_conflicts_of(e_prime));
-  //     });
-
-  // const EventSet Q_CDU = C_union_D.make_union(conflict_union.get_local_config());
-
-  // XBT_DEBUG("Computed Q_CDU as '%s'", Q_CDU.to_string().c_str());
-
-  // // Move {e} \ Q_CDU from U to G
-  // if (not Q_CDU.contains(e)) {
-  //   XBT_DEBUG("Moving %s from U to G...", e->to_string().c_str());
-  //   clean_up_set.insert(e);
-  // }
-
-  // // foreach ê in #ⁱ_U(e)
-  // for (const auto* e_hat : this->unfolding.get_immediate_conflicts_of(e)) {
-  //   // Move [ê] \ Q_CDU from U to G
-  //   const EventSet to_remove = e_hat->get_local_config().subtracting(Q_CDU);
-  //   XBT_DEBUG("Moving {%s} from U to G...", to_remove.to_string().c_str());
-  //   clean_up_set.form_union(to_remove);
-  // }
-  // // this->unfolding.remove(clean_up_set);
+  // The "clean-up set" conceptually represents
+  // those events which will no longer be considered
+  // by UDPOR during its exploration. The concept is
+  // introduced to avoid modification during iteration
+  // over the current unfolding to determine who needs to
+  // be removed. Since sets are unordered, it's quite possible
+  // that e.g. two events `e` and `e'` such that `e < e'`
+  // which are determined eligible for removal are removed
+  // in the order `e` and then `e'`. Determining that `e'`
+  // needs to be removed requires that its history be in
+  // tact to e.g. compute the conflicts with the event.
+  //
+  // Thus, we compute the set and remove all of the events
+  // at once in lieu of removing events while iterating over them.
+  // We can hypothesize that processing the events in reverse
+  // topological order would prevent any issues concerning
+  // the order in which are processed
+  EventSet clean_up_set;
+
+  // Q_(C, D, U) = C u D u U (complicated expression)
+  // See page 9 of "Unfolding-based Partial Order Reduction"
+
+  // "C u D" portion
+  const EventSet C_union_D = C.get_events().make_union(D);
+
+  // "U (complicated expression)" portion
+  const EventSet conflict_union = std::accumulate(
+      C_union_D.begin(), C_union_D.end(), EventSet(), [&](const EventSet acc, const UnfoldingEvent* e_prime) {
+        return acc.make_union(unfolding.get_immediate_conflicts_of(e_prime));
+      });
+
+  const EventSet Q_CDU = C_union_D.make_union(conflict_union.get_local_config());
+
+  XBT_DEBUG("Computed Q_CDU as '%s'", Q_CDU.to_string().c_str());
+
+  // Move {e} \ Q_CDU from U to G
+  if (not Q_CDU.contains(e)) {
+    XBT_DEBUG("Moving %s from U to G...", e->to_string().c_str());
+    clean_up_set.insert(e);
+  }
+
+  // foreach ê in #ⁱ_U(e)
+  for (const auto* e_hat : this->unfolding.get_immediate_conflicts_of(e)) {
+    // Move [ê] \ Q_CDU from U to G
+    const EventSet to_remove = e_hat->get_local_config().subtracting(Q_CDU);
+    XBT_DEBUG("Moving {%s} from U to G...", to_remove.to_string().c_str());
+    clean_up_set.form_union(to_remove);
+  }
+  // TODO: We compute everything... but we don't actually
+  // remove anything yet. This is a workaround until
+  // we figure out how to deal with the fact that the previous
+  // extension sets computed for past configurations
+  // contain events that may be removed from `U`. Perhaps
+  // it would be best to keep them around forever (they
+  // are moved to `G` after all and can be discarded at will,
+  // which means they may never have to be removed at all).
+  //
+  // Of course, the benefit of moving them into the set `G`
+  // is that the computation for immediate conflicts becomes
+  // more efficient (we have to search all of `U` for such conflicts,
+  // and there would be no reason to search those events
+  // that UDPOR has marked as no longer being important)
+  //
+  // this->unfolding.remove(clean_up_set);
 }
 
 RecordTrace UdporChecker::get_record_trace()
index 2bfea8e..b644448 100644 (file)
@@ -142,6 +142,11 @@ bool EventSet::intersects(const History& history) const
   return std::any_of(history.begin(), history.end(), [=](const UnfoldingEvent* e) { return this->contains(e); });
 }
 
+bool EventSet::intersects(const EventSet& other) const
+{
+  return std::any_of(other.begin(), other.end(), [=](const UnfoldingEvent* e) { return this->contains(e); });
+}
+
 EventSet EventSet::get_largest_maximal_subset() const
 {
   const History history(*this);
index a29e25a..ddd875d 100644 (file)
@@ -65,6 +65,7 @@ public:
   bool empty() const;
   bool contains(const UnfoldingEvent*) const;
   bool contains(const History&) const;
+  bool intersects(const EventSet&) const;
   bool intersects(const History&) const;
   bool is_subset_of(const EventSet&) const;
 
index d17274b..f224685 100644 (file)
@@ -53,7 +53,7 @@ EventSet Unfolding::get_immediate_conflicts_of(const UnfoldingEvent* e) const
   EventSet immediate_conflicts;
   for (const auto event : *this) {
     if (event->immediately_conflicts_with(e)) {
-      immediate_conflicts.insert(e);
+      immediate_conflicts.insert(event);
     }
   }
   return immediate_conflicts;
index 8f58486..65ed352 100644 (file)
@@ -27,6 +27,7 @@ public:
   auto cend() const { return this->event_handles.cend(); }
   size_t size() const { return this->event_handles.size(); }
   bool empty() const { return this->event_handles.empty(); }
+  bool contains(const UnfoldingEvent* e) const { return this->event_handles.contains(e); }
 
   void remove(const UnfoldingEvent* e);
   void remove(const EventSet& events);
index a14b1f3..b1d7382 100644 (file)
@@ -6,6 +6,8 @@
 #include "src/mc/explo/udpor/UnfoldingEvent.hpp"
 #include "src/mc/explo/udpor/History.hpp"
 
+#include <xbt/asserts.h>
+#include <xbt/log.h>
 #include <xbt/string.hpp>
 
 namespace simgrid::mc::udpor {
@@ -52,7 +54,7 @@ std::string UnfoldingEvent::to_string() const
   }
   dependencies_string += "]";
 
-  return xbt::string_printf("Actor %ld: %s (%zu dependencies: %s)", associated_transition->aid_,
+  return xbt::string_printf("(%p) Actor %ld: %s (%zu dependencies: %s)", this, associated_transition->aid_,
                             associated_transition->to_string().c_str(), immediate_causes.size(),
                             dependencies_string.c_str());
 }