From 08ff614e08d1489358ebd041eeb834fda9c2b0da Mon Sep 17 00:00:00 2001 From: Roland Godet Date: Tue, 23 Jul 2024 10:56:47 +0200 Subject: [PATCH] feat(planning): support linear sum as action cost --- planning/grpc/server/src/chronicles.rs | 46 +++----------------- planning/planners/src/encode.rs | 12 +---- planning/planning/src/chronicles/concrete.rs | 24 ++-------- planning/planning/src/parsing/mod.rs | 2 +- 4 files changed, 12 insertions(+), 72 deletions(-) diff --git a/planning/grpc/server/src/chronicles.rs b/planning/grpc/server/src/chronicles.rs index 991329bb..359abae6 100644 --- a/planning/grpc/server/src/chronicles.rs +++ b/planning/grpc/server/src/chronicles.rs @@ -895,48 +895,14 @@ impl<'a> ChronicleFactory<'a> { } fn set_cost(&mut self, cost: &Expression) -> Result<(), Error> { - let tpe = from_upf_type(cost.r#type.as_ref(), &self.context.model.get_symbol_table().types) - .with_context(|| format!("Unknown argument type: {0}", cost.r#type))?; - ensure!(tpe.is_numeric()); - - let cost = match kind(cost)? { - ExpressionKind::Constant => { - ensure!(cost.r#type == "up:integer"); - Cost::Fixed(match cost.atom.as_ref().unwrap().content.as_ref().unwrap() { - Content::Int(i) => *i as IntCst, - _ => bail!("Unexpected cost type."), - }) - } - ExpressionKind::Parameter => { - let name = match cost.atom.as_ref().unwrap().content.as_ref().unwrap() { - Content::Symbol(s) => s.clone(), - _ => bail!("Unexpected cost parameter name type."), - }; - let var = self - .env - .parameters - .get(&name) - .with_context(|| format!("Unknown parameter: {name}"))?; - match var { - Variable::Int(var) => Cost::Variable(*var), - _ => bail!("Cost parameter must be an integer variable."), - } + let value = self.reify(cost, None)?; + match value { + Atom::Int(i) => { + self.chronicle.cost = Some(i); + self.chronicle.constraints.push(Constraint::leq(IAtom::ZERO, i)); } - _ => bail!("Unsupported cost expression: {cost:?}"), + _ => bail!("Cost must be an integer value."), }; - - match cost { - Cost::Fixed(cost) => ensure!(cost >= 0, "Cost must be a non-negative integer ({cost})"), - Cost::Variable(..) => match tpe { - Type::Int { lb, .. } => ensure!( - lb >= 0, - "Cost variable must be a non-negative integer (lower bound = {lb})" - ), - _ => bail!("Cost variable must be an integer variable."), - }, - }; - - self.chronicle.cost = Some(cost); Ok(()) } diff --git a/planning/planners/src/encode.rs b/planning/planners/src/encode.rs index e4d5ce4d..e04a6915 100644 --- a/planning/planners/src/encode.rs +++ b/planning/planners/src/encode.rs @@ -429,12 +429,6 @@ pub fn add_metric(pb: &FiniteProblem, model: &mut Model, metric: Metric) -> IAto let mut costs = Vec::with_capacity(8); for (ch_id, ch) in pb.chronicles.iter().enumerate() { if let Some(cost) = &ch.chronicle.cost { - match cost { - Cost::Fixed(c) => assert!(*c >= 0, "A chronicle has a negative cost"), - Cost::Variable(v) => { - assert!(model.domain_of(*v).0 >= 0, "A chronicle could have a negative cost") - } - }; costs.push((ch_id, ch.chronicle.presence, cost)); } } @@ -443,10 +437,8 @@ pub fn add_metric(pb: &FiniteProblem, model: &mut Model, metric: Metric) -> IAto let action_costs: Vec = costs .iter() .map(|&(ch_id, p, cost)| { - let bounds = match cost { - Cost::Fixed(c) => (*c, *c), - Cost::Variable(v) => model.domain_of(*v), - }; + let bounds = model.domain_of(*cost); + assert!(bounds.0 >= 0, "A chronicle could have a negative cost"); model .new_optional_ivar(bounds.0, bounds.1, p, Container::Instance(ch_id).var(VarType::Cost)) .or_zero(p) diff --git a/planning/planning/src/chronicles/concrete.rs b/planning/planning/src/chronicles/concrete.rs index 857c51c6..b094489a 100644 --- a/planning/planning/src/chronicles/concrete.rs +++ b/planning/planning/src/chronicles/concrete.rs @@ -6,7 +6,7 @@ use std::sync::Arc; use crate::chronicles::constraints::Constraint; use crate::chronicles::Fluent; -use aries::core::{IntCst, Lit, VarRef}; +use aries::core::{Lit, VarRef}; use aries::model::lang::linear::{LinearSum, LinearTerm}; use aries::model::lang::*; @@ -280,24 +280,6 @@ impl Substitute for StateVar { } } -/// The cost of a chronicle -#[derive(Clone, Debug)] -pub enum Cost { - /// The cost is a fixed integer value - Fixed(IntCst), - /// The cost is a chronicle's variable - Variable(IVar), -} - -impl Substitute for Cost { - fn substitute(&self, s: &impl Substitution) -> Self { - match self { - Cost::Fixed(x) => Cost::Fixed(*x), - Cost::Variable(x) => Cost::Variable(s.sub_ivar(*x)), - } - } -} - /// Represents an effect on a state variable. /// The effect has a first transition phase `]transition_start, transition_end[` during which the /// value of the state variable is unknown. @@ -511,7 +493,7 @@ pub struct Chronicle { /// expression on the start/end timepoint of these subtasks. pub subtasks: Vec, /// Cost of this chronicle. If left empty, it is interpreted as 0. - pub cost: Option, + pub cost: Option, } struct VarSet(HashSet); @@ -643,7 +625,7 @@ impl Substitute for Chronicle { effects: self.effects.iter().map(|e| e.substitute(s)).collect(), constraints: self.constraints.iter().map(|c| c.substitute(s)).collect(), subtasks: self.subtasks.iter().map(|c| c.substitute(s)).collect(), - cost: self.cost.as_ref().map(|c| c.substitute(s)), + cost: self.cost.map(|c| s.isub(c)), } } } diff --git a/planning/planning/src/parsing/mod.rs b/planning/planning/src/parsing/mod.rs index e5cf8b8b..d659a7b8 100644 --- a/planning/planning/src/parsing/mod.rs +++ b/planning/planning/src/parsing/mod.rs @@ -393,7 +393,7 @@ fn read_chronicle_template( // TODO: here the cost is simply 1 for any primitive action let cost = match pddl.kind() { ChronicleKind::Problem | ChronicleKind::Method => None, - ChronicleKind::Action | ChronicleKind::DurativeAction => Some(Cost::Fixed(1)), + ChronicleKind::Action | ChronicleKind::DurativeAction => Some(1.into()), }; let mut ch = Chronicle {