feat: add method for retrieving relevant brancher events (#138)

For some instances (e.g. `2023/code-generator/unison.mzn`, with
`2023/code-generator/mips_gcc.flow.find_regno_partial.dzn`), it showed
up in profiling that the dynamic brancher went through a lot of
branchers upon certain events (in this case the `on_unassign_integer`
event, even though only a single brancher was interested in these
events).

To alleviate this issue, this PR adds a method for `Brancher`s,
`VariableSelector`s, and `ValueSelector`s which allows them to indicate
which events they are interested in; this allows the `DynamicBrancher`
to only call the `Brancher`s which are interested in it.

Author: ImkoMarijnissen

Cargo.lock

@@ -26,6 +26,7 @@ clap = { version = "4.5.17", features = ["derive"] }
 env_logger = "0.10.0"
 bitfield-struct = "0.9.2"
 num = "0.4.3"
+enum-map = "2.7.3"
 
 [dev-dependencies]
 clap = { version = "4.5.17", features = ["derive"] }

pumpkin-solver/Cargo.toml

@@ -1,9 +1,13 @@
+use enum_map::Enum;
+
 #[cfg(doc)]
 use crate::basic_types::Random;
 use crate::basic_types::SolutionReference;
 #[cfg(doc)]
 use crate::branching;
 #[cfg(doc)]
+use crate::branching::branchers::dynamic_brancher::DynamicBrancher;
+#[cfg(doc)]
 use crate::branching::value_selection::ValueSelector;
 #[cfg(doc)]
 use crate::branching::variable_selection::VariableSelector;
@@ -39,33 +43,53 @@ pub trait Brancher {
 
     /// A function which is called after a conflict has been found and processed but (currently)
     /// does not provide any additional information.
+    ///
+    /// To receive information about this event, use [`BrancherEvent::Conflict`] in
+    /// [`Self::subscribe_to_events`]
     fn on_conflict(&mut self) {}
 
     /// A function which is called whenever a backtrack occurs in the [`Solver`].
+    ///
+    /// To receive information about this event, use [`BrancherEvent::Backtrack`] in
+    /// [`Self::subscribe_to_events`]
     fn on_backtrack(&mut self) {}
 
     /// This method is called when a solution is found; this will either be called when a new
     /// incumbent solution is found (i.e. a solution with a better objective value than previously
     /// known) or when a new solution is found when iterating over solutions using
     /// [`SolutionIterator`].
+    ///
+    /// To receive information about this event, use [`BrancherEvent::Solution`] in
+    /// [`Self::subscribe_to_events`]
     fn on_solution(&mut self, _solution: SolutionReference) {}
 
     /// A function which is called after a [`DomainId`] is unassigned during backtracking (i.e. when
     /// it was fixed but is no longer), specifically, it provides `variable` which is the
     /// [`DomainId`] which has been reset and `value` which is the value to which the variable was
     /// previously fixed. This method could thus be called multiple times in a single
     /// backtracking operation by the solver.
+    ///
+    /// To receive information about this event, use [`BrancherEvent::UnassignInteger`] in
+    /// [`Self::subscribe_to_events`]
     fn on_unassign_integer(&mut self, _variable: DomainId, _value: i32) {}
 
     /// A function which is called when a [`Predicate`] appears in a conflict during conflict
     /// analysis.
+    ///
+    /// To receive information about this event, use
+    /// [`BrancherEvent::AppearanceInConflictPredicate`] in [`Self::subscribe_to_events`]
     fn on_appearance_in_conflict_predicate(&mut self, _predicate: Predicate) {}
 
     /// This method is called whenever a restart is performed.
+    /// To receive information about this event, use [`BrancherEvent::Restart`] in
+    /// [`Self::subscribe_to_events`]
     fn on_restart(&mut self) {}
 
     /// Called after backtracking.
     /// Used to reset internal data structures to account for the backtrack.
+    ///
+    /// To receive information about this event, use [`BrancherEvent::Synchronise`] in
+    /// [`Self::subscribe_to_events`]
     fn synchronise(&mut self, _assignments: &Assignments) {}
 
     /// This method returns whether a restart is *currently* pointless for the [`Brancher`].
@@ -81,4 +105,31 @@ pub trait Brancher {
     fn is_restart_pointless(&mut self) -> bool {
         true
     }
+
+    /// Indicates which [`BrancherEvent`] are relevant for this particular [`Brancher`].
+    ///
+    /// This can be used by [`Brancher::subscribe_to_events`] to determine upon which
+    /// events which [`VariableSelector`] should be called.
+    fn subscribe_to_events(&self) -> Vec<BrancherEvent>;
+}
+
+/// The events which can occur for a [`Brancher`]. Used for returning which events are relevant in
+/// [`Brancher::subscribe_to_events`], [`VariableSelector::subscribe_to_events`],
+/// and [`ValueSelector::subscribe_to_events`].
+#[derive(Debug, Clone, Copy, Enum, Hash, PartialEq, Eq)]
+pub enum BrancherEvent {
+    /// Event for when a conflict is detected
+    Conflict,
+    /// Event for when a backtrack is performed
+    Backtrack,
+    /// Event for when a solution has been found
+    Solution,
+    /// Event for when an integer variable has become unassigned
+    UnassignInteger,
+    /// Event for when a predicate appears during conflict analysis
+    AppearanceInConflictPredicate,
+    /// Event for when a restart occurs
+    Restart,
+    /// Event which is called with the new state after a backtrack has occurred
+    Synchronise,
 }

pumpkin-solver/src/branching/brancher.rs

@@ -2,6 +2,7 @@
 //! on the strategy specified in [`AlternatingStrategy`].
 
 use crate::basic_types::SolutionReference;
+use crate::branching::brancher::BrancherEvent;
 use crate::branching::Brancher;
 use crate::branching::SelectionContext;
 use crate::engine::predicates::predicate::Predicate;
@@ -208,6 +209,16 @@ impl<OtherBrancher: Brancher> Brancher for AlternatingBrancher<OtherBrancher> {
             self.other_brancher.synchronise(assignments);
         }
     }
+
+    fn subscribe_to_events(&self) -> Vec<BrancherEvent> {
+        // We require the restart event and on solution event for the alternating brancher itself;
+        // additionally, it will be interested in the events of its sub-branchers
+        [BrancherEvent::Restart, BrancherEvent::Solution]
+            .into_iter()
+            .chain(self.default_brancher.subscribe_to_events())
+            .chain(self.other_brancher.subscribe_to_events())
+            .collect()
+    }
 }
 
 #[cfg(test)]

pumpkin-solver/src/branching/branchers/alternating_brancher.rs

@@ -5,6 +5,7 @@ use crate::basic_types::SolutionReference;
 use crate::branching::value_selection::RandomSplitter;
 use crate::branching::variable_selection::RandomSelector;
 use crate::branching::Brancher;
+use crate::branching::BrancherEvent;
 use crate::branching::SelectionContext;
 use crate::containers::KeyValueHeap;
 use crate::containers::StorageKey;
@@ -292,6 +293,19 @@ impl<BackupBrancher: Brancher> Brancher for AutonomousSearch<BackupBrancher> {
     fn is_restart_pointless(&mut self) -> bool {
         false
     }
+
+    fn subscribe_to_events(&self) -> Vec<BrancherEvent> {
+        [
+            BrancherEvent::Solution,
+            BrancherEvent::Conflict,
+            BrancherEvent::Backtrack,
+            BrancherEvent::Synchronise,
+            BrancherEvent::AppearanceInConflictPredicate,
+        ]
+        .into_iter()
+        .chain(self.backup_brancher.subscribe_to_events())
+        .collect()
+    }
 }
 
 #[cfg(test)]

pumpkin-solver/src/branching/branchers/autonomous_search.rs

@@ -6,7 +6,11 @@
 use std::cmp::min;
 use std::fmt::Debug;
 
+use enum_map::EnumMap;
+
+use crate::basic_types::HashSet;
 use crate::basic_types::SolutionReference;
+use crate::branching::brancher::BrancherEvent;
 use crate::branching::Brancher;
 use crate::branching::SelectionContext;
 use crate::engine::predicates::predicate::Predicate;
@@ -28,6 +32,9 @@ use crate::engine::Assignments;
 pub struct DynamicBrancher {
     branchers: Vec<Box<dyn Brancher>>,
     brancher_index: usize,
+
+    relevant_event_to_index: EnumMap<BrancherEvent, Vec<usize>>,
+    relevant_events: Vec<BrancherEvent>,
 }
 
 impl Debug for DynamicBrancher {
@@ -40,14 +47,36 @@ impl DynamicBrancher {
     /// Creates a new [`DynamicBrancher`] with the provided `branchers`. It will attempt to use the
     /// `branchers` in the order in which they were provided.
     pub fn new(branchers: Vec<Box<dyn Brancher>>) -> Self {
+        let mut relevant_event_to_index: EnumMap<BrancherEvent, Vec<usize>> = EnumMap::default();
+        let mut relevant_events = HashSet::new();
+
+        // The dynamic brancher will reset the indices upon these events so they should be called
+        let _ = relevant_events.insert(BrancherEvent::Solution);
+        let _ = relevant_events.insert(BrancherEvent::Conflict);
+
+        branchers.iter().enumerate().for_each(|(index, brancher)| {
+            for event in brancher.subscribe_to_events() {
+                relevant_event_to_index[event].push(index);
+                let _ = relevant_events.insert(event);
+            }
+        });
         Self {
             branchers,
             brancher_index: 0,
+
+            relevant_event_to_index,
+            relevant_events: relevant_events.into_iter().collect(),
         }
     }
 
     pub fn add_brancher(&mut self, brancher: Box<dyn Brancher>) {
-        self.branchers.push(brancher)
+        for event in brancher.subscribe_to_events() {
+            self.relevant_event_to_index[event].push(self.branchers.len());
+            if !self.relevant_events.contains(&event) {
+                self.relevant_events.push(event);
+            }
+        }
+        self.branchers.push(brancher);
     }
 }
 
@@ -70,46 +99,50 @@ impl Brancher for DynamicBrancher {
         // A conflict has occurred, we do not know which brancher now can select a variable, reset
         // to the first one
         self.brancher_index = 0;
-        self.branchers
-            .iter_mut()
-            .for_each(|brancher| brancher.on_conflict());
+        self.relevant_event_to_index[BrancherEvent::Conflict]
+            .iter()
+            .for_each(|&brancher_index| self.branchers[brancher_index].on_conflict());
     }
 
     fn on_backtrack(&mut self) {
-        self.branchers
-            .iter_mut()
-            .for_each(|brancher| brancher.on_backtrack());
+        self.relevant_event_to_index[BrancherEvent::Backtrack]
+            .iter()
+            .for_each(|&brancher_index| self.branchers[brancher_index].on_backtrack());
     }
 
     fn on_unassign_integer(&mut self, variable: DomainId, value: i32) {
-        self.branchers
-            .iter_mut()
-            .for_each(|brancher| brancher.on_unassign_integer(variable, value));
+        self.relevant_event_to_index[BrancherEvent::UnassignInteger]
+            .iter()
+            .for_each(|&brancher_index| {
+                self.branchers[brancher_index].on_unassign_integer(variable, value)
+            });
     }
 
     fn on_appearance_in_conflict_predicate(&mut self, predicate: Predicate) {
-        self.branchers
-            .iter_mut()
-            .for_each(|brancher| brancher.on_appearance_in_conflict_predicate(predicate));
+        self.relevant_event_to_index[BrancherEvent::AppearanceInConflictPredicate]
+            .iter()
+            .for_each(|&brancher_index| {
+                self.branchers[brancher_index].on_appearance_in_conflict_predicate(predicate)
+            });
     }
 
     fn on_solution(&mut self, solution: SolutionReference) {
         self.brancher_index = 0;
-        self.branchers
-            .iter_mut()
-            .for_each(|brancher| brancher.on_solution(solution));
+        self.relevant_event_to_index[BrancherEvent::Solution]
+            .iter()
+            .for_each(|&brancher_index| self.branchers[brancher_index].on_solution(solution));
     }
 
     fn on_restart(&mut self) {
-        self.branchers
-            .iter_mut()
-            .for_each(|brancher| brancher.on_restart());
+        self.relevant_event_to_index[BrancherEvent::Restart]
+            .iter()
+            .for_each(|&brancher_index| self.branchers[brancher_index].on_restart());
     }
 
     fn synchronise(&mut self, assignments: &Assignments) {
-        self.branchers
-            .iter_mut()
-            .for_each(|brancher| brancher.synchronise(assignments));
+        self.relevant_event_to_index[BrancherEvent::Synchronise]
+            .iter()
+            .for_each(|&brancher_index| self.branchers[brancher_index].synchronise(assignments));
     }
 
     fn is_restart_pointless(&mut self) -> bool {
@@ -120,4 +153,8 @@ impl Brancher for DynamicBrancher {
             .iter_mut()
             .all(|brancher| brancher.is_restart_pointless())
     }
+
+    fn subscribe_to_events(&self) -> Vec<BrancherEvent> {
+        self.relevant_events.clone()
+    }
 }

pumpkin-solver/src/branching/branchers/dynamic_brancher.rs

@@ -4,6 +4,7 @@
 use std::marker::PhantomData;
 
 use crate::basic_types::SolutionReference;
+use crate::branching::brancher::BrancherEvent;
 use crate::branching::value_selection::ValueSelector;
 use crate::branching::variable_selection::VariableSelector;
 use crate::branching::Brancher;
@@ -89,4 +90,12 @@ where
     fn is_restart_pointless(&mut self) -> bool {
         self.variable_selector.is_restart_pointless() && self.value_selector.is_restart_pointless()
     }
+
+    fn subscribe_to_events(&self) -> Vec<BrancherEvent> {
+        self.variable_selector
+            .subscribe_to_events()
+            .into_iter()
+            .chain(self.value_selector.subscribe_to_events())
+            .collect()
+    }
 }

pumpkin-solver/src/branching/branchers/independent_variable_value_brancher.rs

@@ -71,7 +71,7 @@ pub mod tie_breaking;
 pub mod value_selection;
 pub mod variable_selection;
 
-pub use brancher::Brancher;
+pub use brancher::*;
 pub use selection_context::SelectionContext;
 
 #[cfg(doc)]

pumpkin-solver/src/branching/mod.rs

@@ -2,6 +2,7 @@ use std::fmt::Debug;
 
 use super::ValueSelector;
 use crate::basic_types::SolutionReference;
+use crate::branching::brancher::BrancherEvent;
 #[cfg(doc)]
 use crate::branching::branchers::dynamic_brancher::DynamicBrancher;
 use crate::branching::SelectionContext;
@@ -46,4 +47,8 @@ impl<Var> ValueSelector<Var> for DynamicValueSelector<Var> {
     fn is_restart_pointless(&mut self) -> bool {
         self.selector.is_restart_pointless()
     }
+
+    fn subscribe_to_events(&self) -> Vec<BrancherEvent> {
+        self.selector.subscribe_to_events()
+    }
 }