From e341ae8601fd17d9d1e0698b075dd5abbc6e2436 Mon Sep 17 00:00:00 2001 From: Felix Date: Thu, 11 Apr 2024 18:18:56 +0100 Subject: [PATCH 1/3] iterate all domain values --- .vscode/settings.json | 1 + Cargo.lock | 2 +- conjure_oxide/Cargo.toml | 1 - crates/conjure_core/Cargo.toml | 1 + crates/conjure_core/src/ast/domains.rs | 133 +++++++++------- crates/conjure_core/src/ast/expressions.rs | 158 ++++++++++++++------ crates/conjure_core/src/rule_engine/rule.rs | 4 +- crates/conjure_core/src/rules/base.rs | 11 +- crates/conjure_core/src/rules/minion.rs | 13 +- 9 files changed, 205 insertions(+), 119 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index dbc9673b7c..7a4998e85f 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -7,4 +7,5 @@ "coverage-gutters.coverageFileNames": [ "lcov.info", ], + "workbench.iconTheme": "vs-seti", } diff --git a/Cargo.lock b/Cargo.lock index 96fddbbcac..e9769045ad 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -328,6 +328,7 @@ dependencies = [ "derivative", "derive_is_enum_variant", "enum_compatability_macro", + "itertools", "linkme", "log", "minion_rs", @@ -362,7 +363,6 @@ dependencies = [ "anyhow", "clap", "conjure_core", - "itertools", "linkme", "log", "minion_rs", diff --git a/conjure_oxide/Cargo.toml b/conjure_oxide/Cargo.toml index 6ed4d47113..7cb06fe6d6 100644 --- a/conjure_oxide/Cargo.toml +++ b/conjure_oxide/Cargo.toml @@ -24,7 +24,6 @@ strum = "0.26.2" versions = "6.2.0" linkme = "0.3.22" walkdir = "2.5.0" -itertools = "0.12.1" regex = "1.10.3" log = "0.4.21" structured-logger = "1.0.3" diff --git a/crates/conjure_core/Cargo.toml b/crates/conjure_core/Cargo.toml index 9b8086e8ba..97efa7b0e6 100644 --- a/crates/conjure_core/Cargo.toml +++ b/crates/conjure_core/Cargo.toml @@ -25,6 +25,7 @@ walkdir = "2.5.0" derivative = "2.2.0" schemars = "0.8.16" clap = { version = "4.5.4", features = ["derive"] } +itertools = "0.12.1" [lints] workspace = true diff --git a/crates/conjure_core/src/ast/domains.rs b/crates/conjure_core/src/ast/domains.rs index db4a3fe2a8..e0c8ae84cf 100644 --- a/crates/conjure_core/src/ast/domains.rs +++ b/crates/conjure_core/src/ast/domains.rs @@ -1,7 +1,11 @@ use serde::{Deserialize, Serialize}; +// use std::iter::Ste #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] -pub enum Range { +pub enum Range +where + A: Ord, +{ Single(A), Bounded(A, A), } @@ -13,70 +17,85 @@ pub enum Domain { } impl Domain { - /// Returns the minimum i32 value a variable of the domain can take, if it is an i32 domain. - pub fn min_i32(&self) -> Option { + /// Return a list of all possible i32 values in the domain if it is an IntDomain. + pub fn values_i32(&self) -> Option> { match self { - Domain::BoolDomain => Some(0), - Domain::IntDomain(ranges) => { - if ranges.is_empty() { - return None; - } - let mut min = i32::MAX; - for r in ranges { - match r { - Range::Single(i) => min = min.min(*i), - Range::Bounded(i, _) => min = min.min(*i), - } - } - Some(min) - } + Domain::IntDomain(ranges) => Some( + ranges + .iter() + .flat_map(|r| match r { + Range::Single(i) => vec![*i], + Range::Bounded(i, j) => (*i..=*j).collect(), + }) + .collect(), + ), + _ => None, } } - /// Returns the maximum i32 value a variable of the domain can take, if it is an i32 domain. - pub fn max_i32(&self) -> Option { - match self { - Domain::BoolDomain => Some(1), - Domain::IntDomain(ranges) => { - if ranges.is_empty() { - return None; - } - let mut max = i32::MIN; - for r in ranges { - match r { - Range::Single(i) => max = max.max(*i), - Range::Bounded(_, i) => max = max.max(*i), - } - } - Some(max) + /// Return an unoptimised domain that is the result of applying a binary i32 operation to two domains. + /// + /// The given operator may return None if the operation is not defined for the given arguments. + /// Undefined values will not be included in the resulting domain. + /// + /// Returns None if the domains are not valid for i32 operations. + pub fn apply_i32(&self, op: fn(i32, i32) -> Option, other: &Domain) -> Option { + if let (Some(vs1), Some(vs2)) = (self.values_i32(), other.values_i32()) { + let mut new_ranges = vec![]; + for (v1, v2) in itertools::iproduct!(vs1, vs2) { + op(v1, v2).map(|v| new_ranges.push(Range::Single(v))); } + return Some(Domain::IntDomain(new_ranges)); } + None } +} - /// Returns the minimum and maximum integer values a variable of the domain can take, if it is an integer domain. - pub fn min_max_i32(&self) -> Option<(i32, i32)> { - match self { - Domain::BoolDomain => Some((0, 1)), - Domain::IntDomain(ranges) => { - if ranges.is_empty() { - return None; - } - let mut min = i32::MAX; - let mut max = i32::MIN; - for r in ranges { - match r { - Range::Single(i) => { - min = min.min(*i); - max = max.max(*i); - } - Range::Bounded(i, j) => { - min = min.min(*i); - max = max.max(*j); - } - } - } - Some((min, max)) - } +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_negative_product() { + let d1 = Domain::IntDomain(vec![Range::Bounded(-2, 1)]); + let d2 = Domain::IntDomain(vec![Range::Bounded(-2, 1)]); + let res = d1.apply_i32(|a, b| Some(a * b), &d2).unwrap(); + + if let Domain::IntDomain(ranges) = res { + assert!(!ranges.contains(&Range::Single(-4))); + assert!(!ranges.contains(&Range::Single(-3))); + assert!(ranges.contains(&Range::Single(-2))); + assert!(ranges.contains(&Range::Single(-1))); + assert!(ranges.contains(&Range::Single(0))); + assert!(ranges.contains(&Range::Single(1))); + assert!(ranges.contains(&Range::Single(2))); + assert!(!ranges.contains(&Range::Single(3))); + assert!(ranges.contains(&Range::Single(4))); + } else { + panic!(); + } + } + + #[test] + fn test_negative_div() { + let d1 = Domain::IntDomain(vec![Range::Bounded(-2, 1)]); + let d2 = Domain::IntDomain(vec![Range::Bounded(-2, 1)]); + let res = d1 + .apply_i32(|a, b| if b != 0 { Some(a / b) } else { None }, &d2) + .unwrap(); + + if let Domain::IntDomain(ranges) = res { + assert!(!ranges.contains(&Range::Single(-4))); + assert!(!ranges.contains(&Range::Single(-3))); + assert!(ranges.contains(&Range::Single(-2))); + assert!(ranges.contains(&Range::Single(-1))); + assert!(ranges.contains(&Range::Single(0))); + assert!(ranges.contains(&Range::Single(1))); + assert!(ranges.contains(&Range::Single(2))); + assert!(!ranges.contains(&Range::Single(3))); + assert!(!ranges.contains(&Range::Single(4))); + } else { + panic!(); } } } diff --git a/crates/conjure_core/src/ast/expressions.rs b/crates/conjure_core/src/ast/expressions.rs index e17244f306..2279b735a2 100644 --- a/crates/conjure_core/src/ast/expressions.rs +++ b/crates/conjure_core/src/ast/expressions.rs @@ -12,6 +12,8 @@ use crate::ast::symbol_table::{Name, SymbolTable}; use crate::ast::ReturnType; use crate::metadata::Metadata; +use super::{Domain, Range}; + #[document_compatibility] #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, is_enum_variant, Uniplate)] #[non_exhaustive] @@ -104,55 +106,40 @@ pub enum Expression { AllDiff(Metadata, Vec), } +fn expr_vec_to_domain( + exprs: &Vec, + op: fn(i32, i32) -> Option, + vars: &SymbolTable, +) -> Option { + let domains: Vec> = exprs.iter().map(|e| e.domain_of(vars)).collect(); + domains + .into_iter() + .reduce(|a, b| a.and_then(|x| b.and_then(|y| x.apply_i32(op, &y)))) + .flatten() +} + impl Expression { - pub fn bounds(&self, vars: &SymbolTable) -> Option<(i32, i32)> { + /// Returns the possible values of the expression, recursing to leaf expressions + pub fn domain_of(&self, vars: &SymbolTable) -> Option { // TODO: (flm8) change this to return full Domains rather than just bounds match self { - Expression::Reference(_, name) => vars.get(name).and_then(|v| v.domain.min_max_i32()), - Expression::Constant(_, Constant::Int(i)) => Some((*i, *i)), - Expression::Sum(_, exprs) => { - if exprs.is_empty() { - return None; - } - let (mut min, mut max) = (0, 0); - for e in exprs { - if let Some((e_min, e_max)) = e.bounds(vars) { - min += e_min; - max += e_max; - } else { - return None; - } - } - Some((min, max)) + Expression::Reference(_, name) => Some(vars.get(name)?.domain.clone()), + Expression::Constant(_, Constant::Int(n)) => { + Some(Domain::IntDomain(vec![Range::Single(*n)])) } + Expression::Constant(_, Constant::Bool(_)) => Some(Domain::BoolDomain), + Expression::Sum(_, exprs) => expr_vec_to_domain(exprs, |x, y| Some(x + y), vars), Expression::Min(_, exprs) => { - if exprs.is_empty() { - return None; - } - let bounds = exprs - .iter() - .map(|e| e.bounds(vars)) - .collect::>>()?; - Some(( - bounds.iter().map(|(min, _)| *min).min()?, - bounds.iter().map(|(_, max)| *max).min()?, - )) + expr_vec_to_domain(exprs, |x, y| Some(if x < y { x } else { y }), vars) } Expression::UnsafeDiv(_, a, b) | Expression::SafeDiv(_, a, b) => { - let (a_min, a_max) = a.bounds(vars)?; - let (mut b_min, mut b_max) = b.bounds(vars)?; - if b_min == 0 && b_max == 0 { - return None; - } - if b_min == 0 { - b_min += 1; // Smallest number which avoids division by zero - } - if b_max == 0 { - b_max -= 1; // Largest number which avoids division by zero - } - Some((a_min / b_max, a_max / b_min)) // TODO: calculate bounds considering negativ values - } - _ => todo!(), + a.domain_of(vars)?.apply_i32( + |x, y| if y != 0 { Some(x / y) } else { None }, + &b.domain_of(vars)?, + ) + } + _ => todo!("Calculate domain of {:?}", self), + // TODO: (flm8) Add support for calculating the domains of more expression types } } @@ -441,3 +428,90 @@ impl Display for Expression { } } } + +#[cfg(test)] +mod tests { + use crate::ast::DecisionVariable; + + use super::*; + + #[test] + fn test_domain_of_constant_sum() { + let c1 = Expression::Constant(Metadata::new(), Constant::Int(1)); + let c2 = Expression::Constant(Metadata::new(), Constant::Int(2)); + let sum = Expression::Sum(Metadata::new(), vec![c1.clone(), c2.clone()]); + assert_eq!( + sum.domain_of(&SymbolTable::new()), + Some(Domain::IntDomain(vec![Range::Single(3)])) + ); + } + + #[test] + fn test_domain_of_constant_invalid_type() { + let c1 = Expression::Constant(Metadata::new(), Constant::Int(1)); + let c2 = Expression::Constant(Metadata::new(), Constant::Bool(true)); + let sum = Expression::Sum(Metadata::new(), vec![c1.clone(), c2.clone()]); + assert_eq!(sum.domain_of(&SymbolTable::new()), None); + } + + #[test] + fn test_domain_of_empty_sum() { + let sum = Expression::Sum(Metadata::new(), vec![]); + assert_eq!(sum.domain_of(&SymbolTable::new()), None); + } + + #[test] + fn test_domain_of_reference() { + let reference = Expression::Reference(Metadata::new(), Name::MachineName(0)); + let mut vars = SymbolTable::new(); + vars.insert( + Name::MachineName(0), + DecisionVariable::new(Domain::IntDomain(vec![Range::Single(1)])), + ); + assert_eq!( + reference.domain_of(&vars), + Some(Domain::IntDomain(vec![Range::Single(1)])) + ); + } + + #[test] + fn test_domain_of_reference_not_found() { + let reference = Expression::Reference(Metadata::new(), Name::MachineName(0)); + assert_eq!(reference.domain_of(&SymbolTable::new()), None); + } + + #[test] + fn test_domain_of_reference_sum_single() { + let reference = Expression::Reference(Metadata::new(), Name::MachineName(0)); + let mut vars = SymbolTable::new(); + vars.insert( + Name::MachineName(0), + DecisionVariable::new(Domain::IntDomain(vec![Range::Single(1)])), + ); + let sum = Expression::Sum(Metadata::new(), vec![reference.clone(), reference.clone()]); + assert_eq!( + sum.domain_of(&vars), + Some(Domain::IntDomain(vec![Range::Single(2)])) + ); + } + + #[test] + fn test_domain_of_reference_sum_bounded() { + let reference = Expression::Reference(Metadata::new(), Name::MachineName(0)); + let mut vars = SymbolTable::new(); + vars.insert( + Name::MachineName(0), + DecisionVariable::new(Domain::IntDomain(vec![Range::Bounded(1, 2)])), + ); + let sum = Expression::Sum(Metadata::new(), vec![reference.clone(), reference.clone()]); + if let Domain::IntDomain(ranges) = sum.domain_of(&vars).unwrap() { + assert!(!ranges.contains(&Range::Single(1))); + assert!(ranges.contains(&Range::Single(2))); + assert!(ranges.contains(&Range::Single(3))); + assert!(ranges.contains(&Range::Single(4))); + assert!(!ranges.contains(&Range::Single(5))); + } else { + panic!(); + } + } +} diff --git a/crates/conjure_core/src/rule_engine/rule.rs b/crates/conjure_core/src/rule_engine/rule.rs index fa34246076..755d3dc6a5 100644 --- a/crates/conjure_core/src/rule_engine/rule.rs +++ b/crates/conjure_core/src/rule_engine/rule.rs @@ -12,8 +12,8 @@ pub enum ApplicationError { #[error("Rule is not applicable")] RuleNotApplicable, - #[error("Could not find the min/max bounds for the expression")] - BoundError, + #[error("Could not calculate the expression domain")] + DomainError, } /// The result of applying a rule to an expression. diff --git a/crates/conjure_core/src/rules/base.rs b/crates/conjure_core/src/rules/base.rs index 5399b76bc6..e5fe63da86 100644 --- a/crates/conjure_core/src/rules/base.rs +++ b/crates/conjure_core/src/rules/base.rs @@ -428,13 +428,10 @@ fn min_to_var(expr: &Expr, mdl: &Model) -> ApplicationResult { new_top.push(Expr::Or(Metadata::new(), disjunction)); let mut new_vars = SymbolTable::new(); - let bound = expr - .bounds(&mdl.variables) - .ok_or(ApplicationError::BoundError)?; - new_vars.insert( - new_name.clone(), - DecisionVariable::new(Domain::IntDomain(vec![Range::Bounded(bound.0, bound.1)])), - ); + let domain = expr + .domain_of(&mdl.variables) + .ok_or(ApplicationError::DomainError)?; + new_vars.insert(new_name.clone(), DecisionVariable::new(domain)); Ok(Reduction::new( Expr::Reference(Metadata::new(), new_name), diff --git a/crates/conjure_core/src/rules/minion.rs b/crates/conjure_core/src/rules/minion.rs index 81398e4c9a..b46e827c14 100644 --- a/crates/conjure_core/src/rules/minion.rs +++ b/crates/conjure_core/src/rules/minion.rs @@ -273,15 +273,10 @@ fn flatten_safediv(expr: &Expr, mdl: &Model) -> ApplicationResult { for c in sub.iter_mut() { if let Expr::SafeDiv(_, a, b) = c.clone() { let new_name = mdl.gensym(); - let bound = c - .bounds(&mdl.variables) - .ok_or(ApplicationError::BoundError)?; - new_vars.insert( - new_name.clone(), - DecisionVariable::new(Domain::IntDomain(vec![Range::Bounded( - bound.0, bound.1, - )])), - ); + let domain = c + .domain_of(&mdl.variables) + .ok_or(ApplicationError::DomainError)?; + new_vars.insert(new_name.clone(), DecisionVariable::new(domain)); new_top.push(Expr::DivEq( Metadata::new(), From 223382a7ee734cd96b5ec21e08c0f760504a18fe Mon Sep 17 00:00:00 2001 From: Felix Date: Thu, 11 Apr 2024 18:53:30 +0100 Subject: [PATCH 2/3] (temp) collapse multi-range domains into a single range --- crates/conjure_core/src/ast/domains.rs | 1 + crates/conjure_core/src/ast/expressions.rs | 57 ++++++++++++++++------ 2 files changed, 44 insertions(+), 14 deletions(-) diff --git a/crates/conjure_core/src/ast/domains.rs b/crates/conjure_core/src/ast/domains.rs index e0c8ae84cf..259fd45817 100644 --- a/crates/conjure_core/src/ast/domains.rs +++ b/crates/conjure_core/src/ast/domains.rs @@ -41,6 +41,7 @@ impl Domain { /// Returns None if the domains are not valid for i32 operations. pub fn apply_i32(&self, op: fn(i32, i32) -> Option, other: &Domain) -> Option { if let (Some(vs1), Some(vs2)) = (self.values_i32(), other.values_i32()) { + // TODO: (flm8) Optimise to use smarter, less brute-force methods let mut new_ranges = vec![]; for (v1, v2) in itertools::iproduct!(vs1, vs2) { op(v1, v2).map(|v| new_ranges.push(Range::Single(v))); diff --git a/crates/conjure_core/src/ast/expressions.rs b/crates/conjure_core/src/ast/expressions.rs index 2279b735a2..b56cdf45a4 100644 --- a/crates/conjure_core/src/ast/expressions.rs +++ b/crates/conjure_core/src/ast/expressions.rs @@ -106,7 +106,7 @@ pub enum Expression { AllDiff(Metadata, Vec), } -fn expr_vec_to_domain( +fn expr_vec_to_domain_i32( exprs: &Vec, op: fn(i32, i32) -> Option, vars: &SymbolTable, @@ -118,19 +118,44 @@ fn expr_vec_to_domain( .flatten() } +fn range_vec_bounds_i32(ranges: &Vec>) -> (i32, i32) { + let mut min = i32::MAX; + let mut max = i32::MIN; + for r in ranges { + match r { + Range::Single(i) => { + if *i < min { + min = *i; + } + if *i > max { + max = *i; + } + } + Range::Bounded(i, j) => { + if *i < min { + min = *i; + } + if *j > max { + max = *j; + } + } + } + } + (min, max) +} + impl Expression { /// Returns the possible values of the expression, recursing to leaf expressions pub fn domain_of(&self, vars: &SymbolTable) -> Option { - // TODO: (flm8) change this to return full Domains rather than just bounds - match self { + let ret = match self { Expression::Reference(_, name) => Some(vars.get(name)?.domain.clone()), Expression::Constant(_, Constant::Int(n)) => { Some(Domain::IntDomain(vec![Range::Single(*n)])) } Expression::Constant(_, Constant::Bool(_)) => Some(Domain::BoolDomain), - Expression::Sum(_, exprs) => expr_vec_to_domain(exprs, |x, y| Some(x + y), vars), + Expression::Sum(_, exprs) => expr_vec_to_domain_i32(exprs, |x, y| Some(x + y), vars), Expression::Min(_, exprs) => { - expr_vec_to_domain(exprs, |x, y| Some(if x < y { x } else { y }), vars) + expr_vec_to_domain_i32(exprs, |x, y| Some(if x < y { x } else { y }), vars) } Expression::UnsafeDiv(_, a, b) | Expression::SafeDiv(_, a, b) => { a.domain_of(vars)?.apply_i32( @@ -140,6 +165,15 @@ impl Expression { } _ => todo!("Calculate domain of {:?}", self), // TODO: (flm8) Add support for calculating the domains of more expression types + }; + match ret { + // TODO: (flm8) the Minion bindings currently only support single ranges for domains, so we use the min/max bounds + // Once they support a full domain as we define it, we can remove this conversion + Some(Domain::IntDomain(ranges)) if ranges.len() > 1 => { + let (min, max) = range_vec_bounds_i32(&ranges); + Some(Domain::IntDomain(vec![Range::Bounded(min, max)])) + } + _ => ret, } } @@ -504,14 +538,9 @@ mod tests { DecisionVariable::new(Domain::IntDomain(vec![Range::Bounded(1, 2)])), ); let sum = Expression::Sum(Metadata::new(), vec![reference.clone(), reference.clone()]); - if let Domain::IntDomain(ranges) = sum.domain_of(&vars).unwrap() { - assert!(!ranges.contains(&Range::Single(1))); - assert!(ranges.contains(&Range::Single(2))); - assert!(ranges.contains(&Range::Single(3))); - assert!(ranges.contains(&Range::Single(4))); - assert!(!ranges.contains(&Range::Single(5))); - } else { - panic!(); - } + assert_eq!( + sum.domain_of(&vars), + Some(Domain::IntDomain(vec![Range::Bounded(2, 4)])) + ); } } From 0cfbcf0bcffc085afbaf8608be2c5ceea1657028 Mon Sep 17 00:00:00 2001 From: Felix Date: Thu, 11 Apr 2024 19:02:11 +0100 Subject: [PATCH 3/3] tweak documentation --- crates/conjure_core/src/ast/domains.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/conjure_core/src/ast/domains.rs b/crates/conjure_core/src/ast/domains.rs index 259fd45817..10a5472d75 100644 --- a/crates/conjure_core/src/ast/domains.rs +++ b/crates/conjure_core/src/ast/domains.rs @@ -35,7 +35,7 @@ impl Domain { /// Return an unoptimised domain that is the result of applying a binary i32 operation to two domains. /// - /// The given operator may return None if the operation is not defined for the given arguments. + /// The given operator may return None if the operation is not defined for its arguments. /// Undefined values will not be included in the resulting domain. /// /// Returns None if the domains are not valid for i32 operations.