Merge pull request #624 from gskorokhod/rm-ruleset-order

refactor: Remove RuleSet order

crates/conjure_core/src/rule_engine/mod.rs

@@ -51,15 +51,15 @@ pub use conjure_macros::register_rule;
 /// This macro uses the following syntax:
 ///
 /// ```text
-/// register_rule_set!(<RuleSet name>, <RuleSet order>, (<DependencyRuleSet1>, <DependencyRuleSet2>, ...));
+/// register_rule_set!(<RuleSet name>, (<DependencyRuleSet1>, <DependencyRuleSet2>, ...));
 /// ```
 ///
 /// # Example
 ///
 /// ```rust
 /// use conjure_core::rule_engine::register_rule_set;
 ///
-/// register_rule_set!("MyRuleSet", 10, ("DependencyRuleSet", "AnotherRuleSet"));
+/// register_rule_set!("MyRuleSet", ("DependencyRuleSet", "AnotherRuleSet"));
 /// ```
 #[doc(inline)]
 pub use conjure_macros::register_rule_set;
@@ -159,17 +159,17 @@ pub fn get_rule_by_name(name: &str) -> Option<&'static Rule<'static>> {
 /// use conjure_core::rule_engine::register_rule_set;
 /// use conjure_core::rule_engine::get_rule_sets;
 ///
-/// register_rule_set!("MyRuleSet", 10, ("AnotherRuleSet"));
-/// register_rule_set!("AnotherRuleSet", 5, ());
+/// register_rule_set!("MyRuleSet", ("AnotherRuleSet"));
+/// register_rule_set!("AnotherRuleSet", ());
 ///
 /// println!("Rule sets: {:?}", get_rule_sets());
 /// ```
 ///
 /// This will print (if no other rule sets are registered):
 /// ```text
 /// Rule sets: [
-///   RuleSet { name: "MyRuleSet", order: 10, rules: OnceLock { state: Uninitialized }, dependencies: ["AnotherRuleSet"] },
-///   RuleSet { name: "AnotherRuleSet", order: 5, rules: OnceLock { state: Uninitialized }, dependencies: [] }
+///   RuleSet { name: "MyRuleSet", rules: OnceLock { state: Uninitialized }, dependencies: ["AnotherRuleSet"] },
+///   RuleSet { name: "AnotherRuleSet", rules: OnceLock { state: Uninitialized }, dependencies: [] }
 /// ]
 /// ```
 ///
@@ -185,14 +185,14 @@ pub fn get_rule_sets() -> Vec<&'static RuleSet<'static>> {
 /// use conjure_core::rule_engine::register_rule_set;
 /// use conjure_core::rule_engine::get_rule_set_by_name;
 ///
-/// register_rule_set!("MyRuleSet", 10, ("DependencyRuleSet", "AnotherRuleSet"));
+/// register_rule_set!("MyRuleSet", ("DependencyRuleSet", "AnotherRuleSet"));
 ///
 /// println!("Rule set: {:?}", get_rule_set_by_name("MyRuleSet"));
 /// ```
 ///
 /// This will print:
 /// ```text
-/// Rule set: Some(RuleSet { name: "MyRuleSet", order: 10, rules: OnceLock { state: Uninitialized }, dependencies: ["DependencyRuleSet", "AnotherRuleSet"] })
+/// Rule set: Some(RuleSet { name: "MyRuleSet", rules: OnceLock { state: Uninitialized }, dependencies: ["DependencyRuleSet", "AnotherRuleSet"] })
 /// ```
 pub fn get_rule_set_by_name(name: &str) -> Option<&'static RuleSet<'static>> {
     get_rule_sets()

crates/conjure_core/src/rule_engine/resolve_rules.rs

@@ -1,6 +1,7 @@
 use std::collections::{HashMap, HashSet};
 use std::fmt::Display;
 
+use log::warn;
 use thiserror::Error;
 
 use crate::rule_engine::{get_rule_set_by_name, get_rule_sets_for_solver_family, Rule, RuleSet};
@@ -94,10 +95,8 @@ pub fn get_rule_priorities<'a>(
 
     for rs in rule_sets {
         for (rule, priority) in rs.get_rules() {
-            if let Some((old_rs, _)) = rule_priorities.get(rule) {
-                if rs.order >= old_rs.order {
-                    rule_priorities.insert(rule, (rs, *priority));
-                }
+            if rule_priorities.contains_key(rule) {
+                warn!("Rule {} is defined in multiple rule sets.", rule.name);
             } else {
                 rule_priorities.insert(rule, (rs, *priority));
             }

crates/conjure_core/src/rule_engine/rule_set.rs

@@ -31,9 +31,6 @@ use crate::solver::SolverFamily;
 pub struct RuleSet<'a> {
     /// The name of the rule set.
     pub name: &'a str,
-    /// Order of the RuleSet. Used to establish a consistent order of operations when resolving rules.
-    /// If two RuleSets overlap (contain the same rule but with different priorities), the RuleSet with the higher order will be used as the source of truth.
-    pub order: u16,
     /// A map of rules to their priorities. This will be lazily initialized at runtime.
     rules: OnceLock<HashMap<&'a Rule<'a>, u16>>,
     /// The names of the rule sets that this rule set depends on.
@@ -46,13 +43,11 @@ pub struct RuleSet<'a> {
 impl<'a> RuleSet<'a> {
     pub const fn new(
         name: &'a str,
-        order: u16,
         dependencies: &'a [&'a str],
         solver_families: &'a [SolverFamily],
     ) -> Self {
         Self {
             name,
-            order,
             dependency_rs_names: dependencies,
             solver_families,
             rules: OnceLock::new(),
@@ -181,11 +176,10 @@ impl Display for RuleSet<'_> {
             f,
             "RuleSet {{\n\
             \tname: {}\n\
-            \torder: {}\n\
             \trules: {}\n\
             \tsolver_families: {:?}\n\
         }}",
-            self.name, self.order, n_rules, solver_families
+            self.name, n_rules, solver_families
         )
     }
 }

crates/conjure_core/src/rules/base.rs

@@ -10,7 +10,7 @@ use Atom::*;
 use Expr::*;
 use Lit::*;
 
-register_rule_set!("Base", 100, ());
+register_rule_set!("Base", ());
 
 /// This rule simplifies expressions where the operator is applied to an empty set of sub-expressions.
 ///

crates/conjure_core/src/rules/bubble.rs

@@ -11,7 +11,7 @@ use crate::ast::{Atom, Literal};
 
 use super::utils::is_all_constant;
 
-register_rule_set!("Bubble", 100, ("Base"));
+register_rule_set!("Bubble", ("Base"));
 
 // Bubble reduction rules
 

crates/conjure_core/src/rules/cnf.rs

@@ -5,4 +5,4 @@
 use conjure_core::rule_engine::register_rule_set;
 use conjure_core::solver::SolverFamily;
 
-register_rule_set!("CNF", 100, ("Base"), (SolverFamily::SAT));
+register_rule_set!("CNF", ("Base"), (SolverFamily::SAT));

crates/conjure_core/src/rules/constant_eval.rs

@@ -8,7 +8,7 @@ use conjure_core::rule_engine::{
 use conjure_core::Model;
 use itertools::izip;
 
-register_rule_set!("Constant", 100, ());
+register_rule_set!("Constant", ());
 
 #[register_rule(("Constant", 9001))]
 fn apply_eval_constant(expr: &Expr, _: &Model) -> ApplicationResult {

crates/conjure_core/src/rules/minion.rs

@@ -19,7 +19,7 @@ use ApplicationError::*;
 
 use super::utils::{is_flat, to_aux_var};
 
-register_rule_set!("Minion", 100, ("Base"), (SolverFamily::Minion));
+register_rule_set!("Minion", ("Base"), (SolverFamily::Minion));
 
 #[register_rule(("Minion", 4200))]
 fn introduce_producteq(expr: &Expr, model: &Model) -> ApplicationResult {

crates/conjure_macros/src/lib.rs

@@ -98,21 +98,17 @@ fn parse_parenthesized<T: Parse>(input: ParseStream) -> Result<Vec<T>> {
 
 struct RuleSetArgs {
     name: LitStr,
-    priority: LitInt,
     dependencies: Vec<LitStr>,
     solver_families: Vec<Path>,
 }
 
 impl Parse for RuleSetArgs {
     fn parse(input: ParseStream) -> Result<Self> {
         let name = input.parse()?;
-        input.parse::<Comma>()?;
-        let priority = input.parse()?;
 
         if input.is_empty() {
             return Ok(Self {
                 name,
-                priority,
                 dependencies: Vec::new(),
                 solver_families: Vec::new(),
             });
@@ -124,7 +120,6 @@ impl Parse for RuleSetArgs {
         if input.is_empty() {
             return Ok(Self {
                 name,
-                priority,
                 dependencies,
                 solver_families: Vec::new(),
             });
@@ -135,27 +130,25 @@ impl Parse for RuleSetArgs {
 
         Ok(Self {
             name,
-            priority,
             dependencies,
             solver_families,
         })
     }
 }
 
 /**
-* Register a rule set with the given name, priority, and dependencies.
+* Register a rule set with the given name, dependencies, and metadata.
 *
 * # Example
 * ```rust
  * use conjure_macros::register_rule_set;
- * register_rule_set!("MyRuleSet", 10, ("DependencyRuleSet", "AnotherRuleSet"));
+ * register_rule_set!("MyRuleSet", ("DependencyRuleSet", "AnotherRuleSet"));
 * ```
  */
 #[proc_macro]
 pub fn register_rule_set(args: TokenStream) -> TokenStream {
     let RuleSetArgs {
         name,
-        priority,
         dependencies,
         solver_families,
     } = parse_macro_input!(args as RuleSetArgs);
@@ -174,7 +167,7 @@ pub fn register_rule_set(args: TokenStream) -> TokenStream {
     let expanded = quote! {
         use ::conjure_core::rule_engine::_dependencies::*; // ToDo idk if we need to explicitly do that?
         #[::conjure_core::rule_engine::_dependencies::distributed_slice(::conjure_core::rule_engine::RULE_SETS_DISTRIBUTED_SLICE)]
-        pub static #static_ident: ::conjure_core::rule_engine::RuleSet<'static> = ::conjure_core::rule_engine::RuleSet::new(#name, #priority, &[#dependencies], &[#solver_families]);
+        pub static #static_ident: ::conjure_core::rule_engine::RuleSet<'static> = ::conjure_core::rule_engine::RuleSet::new(#name, &[#dependencies], &[#solver_families]);
     };
 
     TokenStream::from(expanded)