From b9c737905bd487109063616f8b46663f7912dd0f Mon Sep 17 00:00:00 2001 From: Maarten Flippo Date: Thu, 23 Apr 2026 15:31:26 +1000 Subject: [PATCH 1/7] feat(pumpkin-core): Define an order among predicates The chosen order makes minimizing a sorted collection easy. It can be done in two linear passes, one forward and one backward. --- .../core/src/engine/predicates/predicate.rs | 127 ++++++++++++++++-- 1 file changed, 114 insertions(+), 13 deletions(-) diff --git a/pumpkin-crates/core/src/engine/predicates/predicate.rs b/pumpkin-crates/core/src/engine/predicates/predicate.rs index df6edac0f..1aae8127a 100644 --- a/pumpkin-crates/core/src/engine/predicates/predicate.rs +++ b/pumpkin-crates/core/src/engine/predicates/predicate.rs @@ -10,6 +10,15 @@ use crate::propagation::DomainEvent; /// ([`DomainId`], [`PredicateType`], value). /// /// To create a [`Predicate`], use [Predicate::new] or the more concise [predicate!] macro. +/// ## Order +/// Predicates have a well-defined order. They are first ordered by the domain, and then by +/// predicate type, and finally by the value. The order is chosen such that for a fixed domain `x`, +/// predicates are ordered as follows: +/// [>= 5], [>= 7], [!= 2], [!= 3], [== 5], [!= 7], [<= 6], [<= 10] +/// +/// From the order, we get the lower-bound predicates first, ordered by ascending bound, then the +/// (not-)equal predicates, ordered by ascending bound, then the upper-bound predicates, ordered by +/// descending bounds. #[derive(Clone, PartialEq, Eq, Copy, Hash)] pub struct Predicate { /// The two most significant bits of the id stored in the [`Predicate`] contains the type of @@ -18,21 +27,16 @@ pub struct Predicate { value: i32, } -const LOWER_BOUND_CODE: u8 = 0; -const UPPER_BOUND_CODE: u8 = 1; -const NOT_EQUAL_CODE: u8 = 2; -const EQUAL_CODE: u8 = 3; +const LOWER_BOUND_CODE: u8 = PredicateType::LowerBound as u8; +const UPPER_BOUND_CODE: u8 = PredicateType::UpperBound as u8; +const NOT_EQUAL_CODE: u8 = PredicateType::NotEqual as u8; +const EQUAL_CODE: u8 = PredicateType::Equal as u8; impl Predicate { /// Creates a new [`Predicate`] (also known as atomic constraint) which represents a domain /// operation. pub fn new(id: DomainId, predicate_type: PredicateType, value: i32) -> Self { - let code = match predicate_type { - PredicateType::LowerBound => LOWER_BOUND_CODE, - PredicateType::UpperBound => UPPER_BOUND_CODE, - PredicateType::NotEqual => NOT_EQUAL_CODE, - PredicateType::Equal => EQUAL_CODE, - }; + let code = predicate_type as u8; let id = id.id() | (code as u32) << 30; Self { id, value } } @@ -44,6 +48,39 @@ impl Predicate { pub fn get_predicate_type(&self) -> PredicateType { (*self).into() } + + fn is_bound_predicate(&self) -> bool { + self.is_upper_bound_predicate() || self.is_lower_bound_predicate() + } +} + +impl PartialOrd for Predicate { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for Predicate { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + match self.get_domain().cmp(&other.get_domain()) { + std::cmp::Ordering::Equal => { + if self.is_bound_predicate() || other.is_bound_predicate() { + match self.get_type_code().cmp(&other.get_type_code()) { + std::cmp::Ordering::Equal => { + self.get_right_hand_side().cmp(&other.get_right_hand_side()) + } + ordering @ (std::cmp::Ordering::Less | std::cmp::Ordering::Greater) => { + ordering + } + } + } else { + self.get_right_hand_side().cmp(&other.get_right_hand_side()) + } + } + + ordering @ (std::cmp::Ordering::Less | std::cmp::Ordering::Greater) => ordering, + } + } } #[derive(Debug, Hash, EnumSetType)] @@ -53,9 +90,9 @@ pub enum PredicateType { // Should correspond with the codes defined previously; `EnumSetType` requires that literals // are used and not expressions LowerBound = 0, - UpperBound = 1, - NotEqual = 2, - Equal = 3, + NotEqual = 1, + Equal = 2, + UpperBound = 3, } impl From for PredicateType { @@ -321,4 +358,68 @@ mod test { let trivially_false = Predicate::trivially_false(); assert!(!trivially_false == trivially_true); } + + #[test] + fn predicates_over_same_domain_are_ordered_by_increasing_lower_bound() { + let x = DomainId::new(0); + let p1 = predicate![x >= 4]; + let p2 = predicate![x >= 6]; + assert!(p1 < p2); + } + + #[test] + fn not_equal_predicates_are_bigger_than_lower_bounds() { + let x = DomainId::new(0); + let p1 = predicate![x >= 4]; + let p2 = predicate![x != 6]; + let p3 = predicate![x != 2]; + + assert!(p1 < p2); + assert!(p1 < p3); + } + + #[test] + fn not_equal_predicates_are_ordered_by_rhs() { + let x = DomainId::new(0); + let p1 = predicate![x != 6]; + let p2 = predicate![x != 2]; + + assert!(p1 > p2); + } + + #[test] + fn equal_predicates_are_ordered_by_rhs() { + let x = DomainId::new(0); + let p1 = predicate![x == 6]; + let p2 = predicate![x == 2]; + + assert!(p1 > p2); + } + + #[test] + fn equal_predicates_bigger_than_lower_bounds() { + let x = DomainId::new(0); + let p1 = predicate![x == 6]; + let p2 = predicate![x >= 2]; + + assert!(p1 > p2); + } + + #[test] + fn equal_predicates_smaller_than_upper_bounds() { + let x = DomainId::new(0); + let p1 = predicate![x == 6]; + let p2 = predicate![x <= 2]; + + assert!(p1 < p2); + } + + #[test] + fn tighter_upper_bound_is_smaller() { + let x = DomainId::new(0); + let p1 = predicate![x <= 6]; + let p2 = predicate![x <= 2]; + + assert!(p1 > p2); + } } From 45fac123b7ed70e5c332b9301db38e770ee006ef Mon Sep 17 00:00:00 2001 From: Maarten Flippo Date: Thu, 23 Apr 2026 15:43:42 +1000 Subject: [PATCH 2/7] Implement Predicate::implies --- .../predicate_tracker.rs | 12 +- .../core/src/engine/predicates/predicate.rs | 147 ++++++++++++++++++ 2 files changed, 153 insertions(+), 6 deletions(-) diff --git a/pumpkin-crates/core/src/engine/notifications/predicate_notification/predicate_tracker.rs b/pumpkin-crates/core/src/engine/notifications/predicate_notification/predicate_tracker.rs index 12da90c00..f69ada14e 100644 --- a/pumpkin-crates/core/src/engine/notifications/predicate_notification/predicate_tracker.rs +++ b/pumpkin-crates/core/src/engine/notifications/predicate_notification/predicate_tracker.rs @@ -1087,7 +1087,7 @@ mod tests { vec![ PredicateType::LowerBound, PredicateType::NotEqual, - PredicateType::Equal + PredicateType::Equal, ] ); assert_eq!(value.get_value(), x); @@ -1097,9 +1097,9 @@ mod tests { value.get_predicate_types().collect::>(), vec![ PredicateType::LowerBound, - PredicateType::UpperBound, PredicateType::NotEqual, - PredicateType::Equal + PredicateType::Equal, + PredicateType::UpperBound, ] ); assert_eq!(value.get_value(), x); @@ -1137,7 +1137,7 @@ mod tests { vec![ PredicateType::LowerBound, PredicateType::NotEqual, - PredicateType::Equal + PredicateType::Equal, ] ); assert_eq!(value.get_value(), x); @@ -1147,9 +1147,9 @@ mod tests { value.get_predicate_types().collect::>(), vec![ PredicateType::LowerBound, - PredicateType::UpperBound, PredicateType::NotEqual, - PredicateType::Equal + PredicateType::Equal, + PredicateType::UpperBound, ] ); assert_eq!(value.get_value(), x); diff --git a/pumpkin-crates/core/src/engine/predicates/predicate.rs b/pumpkin-crates/core/src/engine/predicates/predicate.rs index 1aae8127a..713ff2d04 100644 --- a/pumpkin-crates/core/src/engine/predicates/predicate.rs +++ b/pumpkin-crates/core/src/engine/predicates/predicate.rs @@ -41,6 +41,47 @@ impl Predicate { Self { id, value } } + /// Returns `self` if this implies `other`. + pub fn implies(&self, other: Predicate) -> bool { + if self.get_domain() != other.get_domain() { + // Predicates only imply other predicates on the same domain. + return false; + } + + match self.get_predicate_type() { + PredicateType::LowerBound => match other.get_predicate_type() { + PredicateType::LowerBound => { + self.get_right_hand_side() >= other.get_right_hand_side() + } + PredicateType::NotEqual => self.get_right_hand_side() > other.get_right_hand_side(), + PredicateType::UpperBound | PredicateType::Equal => false, + }, + PredicateType::UpperBound => match other.get_predicate_type() { + PredicateType::UpperBound => { + self.get_right_hand_side() <= other.get_right_hand_side() + } + PredicateType::NotEqual => self.get_right_hand_side() < other.get_right_hand_side(), + PredicateType::LowerBound | PredicateType::Equal => false, + }, + PredicateType::NotEqual => { + other.get_predicate_type() == PredicateType::NotEqual + && self.get_right_hand_side() == other.get_right_hand_side() + } + PredicateType::Equal => match other.get_predicate_type() { + PredicateType::LowerBound => { + self.get_right_hand_side() >= other.get_right_hand_side() + } + PredicateType::UpperBound => { + self.get_right_hand_side() <= other.get_right_hand_side() + } + PredicateType::NotEqual => { + self.get_right_hand_side() != other.get_right_hand_side() + } + PredicateType::Equal => self.get_right_hand_side() == other.get_right_hand_side(), + }, + } + } + fn get_type_code(&self) -> u8 { (self.id >> 30) as u8 } @@ -422,4 +463,110 @@ mod test { assert!(p1 > p2); } + + #[test] + fn implies_over_different_domains_is_false() { + let x = DomainId::new(0); + let y = DomainId::new(1); + + assert!(!predicate![x >= 5].implies(predicate![y >= 4])); + } + + #[test] + fn lower_bound_implies() { + let x = DomainId::new(0); + + // Implies weaker bounds + assert!(predicate![x >= 5].implies(predicate![x >= 5])); + assert!(predicate![x >= 5].implies(predicate![x >= 4])); + + // Implies not-equals below bound + assert!(predicate![x >= 5].implies(predicate![x != 4])); + assert!(predicate![x >= 5].implies(predicate![x != 3])); + + // Does not imply stronger bounds + assert!(!predicate![x >= 5].implies(predicate![x >= 6])); + + // Does not imply not-equals at or above bound + assert!(!predicate![x >= 5].implies(predicate![x != 6])); + assert!(!predicate![x >= 5].implies(predicate![x != 5])); + + // Does not imply equals + assert!(!predicate![x >= 5].implies(predicate![x == 6])); + assert!(!predicate![x >= 5].implies(predicate![x == 5])); + assert!(!predicate![x >= 5].implies(predicate![x == 4])); + } + + #[test] + fn upper_bound_implies() { + let x = DomainId::new(0); + + // Implies weaker bounds + assert!(predicate![x <= 5].implies(predicate![x <= 5])); + assert!(predicate![x <= 5].implies(predicate![x <= 6])); + + // Implies not-equals above bound + assert!(predicate![x <= 5].implies(predicate![x != 6])); + assert!(predicate![x <= 5].implies(predicate![x != 7])); + + // Does not imply stronger bounds + assert!(!predicate![x <= 5].implies(predicate![x <= 4])); + + // Does not imply not-equals at or below bound + assert!(!predicate![x <= 5].implies(predicate![x != 4])); + assert!(!predicate![x <= 5].implies(predicate![x != 5])); + + // Does not imply equals + assert!(!predicate![x <= 5].implies(predicate![x == 6])); + assert!(!predicate![x <= 5].implies(predicate![x == 5])); + assert!(!predicate![x <= 5].implies(predicate![x == 4])); + } + + #[test] + fn equals_implies() { + let x = DomainId::new(0); + + // Implies lower bounds at or below + assert!(predicate![x == 5].implies(predicate![x >= 5])); + assert!(predicate![x == 5].implies(predicate![x >= 4])); + + // Implies upper bounds at or above + assert!(predicate![x == 5].implies(predicate![x <= 5])); + assert!(predicate![x == 5].implies(predicate![x <= 6])); + + // Implies not-equals + assert!(predicate![x == 5].implies(predicate![x != 4])); + assert!(predicate![x == 5].implies(predicate![x != 6])); + + // Does not imply not-equals at bound + assert!(!predicate![x == 5].implies(predicate![x != 5])); + + // Does not lower bounds above value + assert!(!predicate![x == 5].implies(predicate![x >= 6])); + + // Does not upper bounds below value + assert!(!predicate![x == 5].implies(predicate![x <= 4])); + } + + #[test] + fn not_equals_implies_nothing() { + let x = DomainId::new(0); + + assert!(!predicate![x != 5].implies(predicate![x <= 4])); + assert!(!predicate![x != 5].implies(predicate![x <= 5])); + assert!(!predicate![x != 5].implies(predicate![x <= 6])); + + assert!(!predicate![x != 5].implies(predicate![x >= 4])); + assert!(!predicate![x != 5].implies(predicate![x >= 5])); + assert!(!predicate![x != 5].implies(predicate![x >= 6])); + + assert!(!predicate![x != 5].implies(predicate![x == 4])); + assert!(!predicate![x != 5].implies(predicate![x == 5])); + assert!(!predicate![x != 5].implies(predicate![x == 6])); + + assert!(!predicate![x != 5].implies(predicate![x != 4])); + assert!(!predicate![x != 5].implies(predicate![x != 6])); + + assert!(predicate![x != 5].implies(predicate![x != 5])); + } } From 6a36fc8a073f091529021c1b8972f84714b96310 Mon Sep 17 00:00:00 2001 From: Maarten Flippo Date: Thu, 23 Apr 2026 15:45:25 +1000 Subject: [PATCH 3/7] Describe order in new paragraph in Predicate docs --- pumpkin-crates/core/src/engine/predicates/predicate.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/pumpkin-crates/core/src/engine/predicates/predicate.rs b/pumpkin-crates/core/src/engine/predicates/predicate.rs index 713ff2d04..dff8bf2bd 100644 --- a/pumpkin-crates/core/src/engine/predicates/predicate.rs +++ b/pumpkin-crates/core/src/engine/predicates/predicate.rs @@ -10,6 +10,7 @@ use crate::propagation::DomainEvent; /// ([`DomainId`], [`PredicateType`], value). /// /// To create a [`Predicate`], use [Predicate::new] or the more concise [predicate!] macro. +/// /// ## Order /// Predicates have a well-defined order. They are first ordered by the domain, and then by /// predicate type, and finally by the value. The order is chosen such that for a fixed domain `x`, From 8958e7566963e6c036f9fd23a7ef97540ffc001a Mon Sep 17 00:00:00 2001 From: Maarten Flippo Date: Wed, 29 Apr 2026 18:02:30 +1000 Subject: [PATCH 4/7] Fixup documentation --- .../core/src/engine/predicates/predicate.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/pumpkin-crates/core/src/engine/predicates/predicate.rs b/pumpkin-crates/core/src/engine/predicates/predicate.rs index dff8bf2bd..5c109cbec 100644 --- a/pumpkin-crates/core/src/engine/predicates/predicate.rs +++ b/pumpkin-crates/core/src/engine/predicates/predicate.rs @@ -17,9 +17,9 @@ use crate::propagation::DomainEvent; /// predicates are ordered as follows: /// [>= 5], [>= 7], [!= 2], [!= 3], [== 5], [!= 7], [<= 6], [<= 10] /// -/// From the order, we get the lower-bound predicates first, ordered by ascending bound, then the -/// (not-)equal predicates, ordered by ascending bound, then the upper-bound predicates, ordered by -/// descending bounds. +/// From the order, we get the lower-bound predicates first, ordered by non-decreasing bound, then the +/// (not-)equal predicates, ordered by non-decreasing bound, then the upper-bound predicates, ordered by +/// non-increasing bounds. #[derive(Clone, PartialEq, Eq, Copy, Hash)] pub struct Predicate { /// The two most significant bits of the id stored in the [`Predicate`] contains the type of @@ -42,7 +42,18 @@ impl Predicate { Self { id, value } } - /// Returns `self` if this implies `other`. + /// Returns `true` if this implies `other`. + /// + /// # Example + /// ``` + /// # use pumpkin_core::variables::DomainId; + /// # use pumpkin_core::predicate; + /// let x = DomainId::new(0); + /// + /// assert!(predicate![x >= 5].implies(predicate![x >= 3])); + /// assert!(predicate![x >= 5].implies(predicate![x != 1])); + /// assert!(predicate![x == 5].implies(predicate![x <= 5])); + /// ``` pub fn implies(&self, other: Predicate) -> bool { if self.get_domain() != other.get_domain() { // Predicates only imply other predicates on the same domain. From e74924ef98afdc0f85d4799a23b5ed803dd1a6d0 Mon Sep 17 00:00:00 2001 From: Maarten Flippo Date: Wed, 29 Apr 2026 18:08:25 +1000 Subject: [PATCH 5/7] Document the `Ord` implementation --- pumpkin-crates/core/src/engine/predicates/predicate.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/pumpkin-crates/core/src/engine/predicates/predicate.rs b/pumpkin-crates/core/src/engine/predicates/predicate.rs index 5c109cbec..2a1e8acff 100644 --- a/pumpkin-crates/core/src/engine/predicates/predicate.rs +++ b/pumpkin-crates/core/src/engine/predicates/predicate.rs @@ -114,6 +114,7 @@ impl PartialOrd for Predicate { } impl Ord for Predicate { + /// See [`Predicate`] for details on the order. fn cmp(&self, other: &Self) -> std::cmp::Ordering { match self.get_domain().cmp(&other.get_domain()) { std::cmp::Ordering::Equal => { From 663bd38e8c0fb2124e9837453ded28aff45858ae Mon Sep 17 00:00:00 2001 From: Maarten Flippo Date: Wed, 29 Apr 2026 18:12:13 +1000 Subject: [PATCH 6/7] Fix formatting --- pumpkin-crates/core/src/engine/predicates/predicate.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pumpkin-crates/core/src/engine/predicates/predicate.rs b/pumpkin-crates/core/src/engine/predicates/predicate.rs index 2a1e8acff..3a2642e61 100644 --- a/pumpkin-crates/core/src/engine/predicates/predicate.rs +++ b/pumpkin-crates/core/src/engine/predicates/predicate.rs @@ -17,9 +17,9 @@ use crate::propagation::DomainEvent; /// predicates are ordered as follows: /// [>= 5], [>= 7], [!= 2], [!= 3], [== 5], [!= 7], [<= 6], [<= 10] /// -/// From the order, we get the lower-bound predicates first, ordered by non-decreasing bound, then the -/// (not-)equal predicates, ordered by non-decreasing bound, then the upper-bound predicates, ordered by -/// non-increasing bounds. +/// From the order, we get the lower-bound predicates first, ordered by non-decreasing bound, then +/// the (not-)equal predicates, ordered by non-decreasing bound, then the upper-bound predicates, +/// ordered by non-increasing bounds. #[derive(Clone, PartialEq, Eq, Copy, Hash)] pub struct Predicate { /// The two most significant bits of the id stored in the [`Predicate`] contains the type of From e6c5572970e18f507bcba07f84e90458595805d8 Mon Sep 17 00:00:00 2001 From: Maarten Flippo Date: Thu, 30 Apr 2026 09:55:00 +1000 Subject: [PATCH 7/7] Replace 'this' with 'self' in documentation --- pumpkin-crates/core/src/engine/predicates/predicate.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pumpkin-crates/core/src/engine/predicates/predicate.rs b/pumpkin-crates/core/src/engine/predicates/predicate.rs index 3a2642e61..b62ae72e1 100644 --- a/pumpkin-crates/core/src/engine/predicates/predicate.rs +++ b/pumpkin-crates/core/src/engine/predicates/predicate.rs @@ -42,7 +42,7 @@ impl Predicate { Self { id, value } } - /// Returns `true` if this implies `other`. + /// Returns `true` if `self` implies `other`. /// /// # Example /// ```