From 9a91995d4f862644a38178dd896116caa547709b Mon Sep 17 00:00:00 2001
From: Roland Godet <r.godet@outlook.fr>
Date: Wed, 11 Sep 2024 09:54:10 +0200
Subject: [PATCH] fix(planning): link cost presence to chronicle presence

---
 planning/grpc/server/src/chronicles.rs | 10 +++++++--
 planning/planners/src/encode.rs        | 28 ++++++++++++--------------
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/planning/grpc/server/src/chronicles.rs b/planning/grpc/server/src/chronicles.rs
index 93bc5289..42c15fe7 100644
--- a/planning/grpc/server/src/chronicles.rs
+++ b/planning/grpc/server/src/chronicles.rs
@@ -1,6 +1,6 @@
 use anyhow::{anyhow, bail, ensure, Context, Error};
 use aries::core::{IntCst, Lit, INT_CST_MAX, INT_CST_MIN};
-use aries::model::extensions::Shaped;
+use aries::model::extensions::{AssignmentExt, Shaped};
 use aries::model::lang::linear::LinearSum;
 use aries::model::lang::*;
 use aries::model::symbols::{SymId, SymbolTable};
@@ -898,7 +898,13 @@ impl<'a> ChronicleFactory<'a> {
         let value = self.reify(cost, None)?;
         match value {
             Atom::Int(i) => {
-                self.chronicle.cost = Some(i);
+                let label = self.container / VarType::Cost;
+                let (lb, ub) = self.context.model.domain_of(i);
+                let var = self
+                    .context
+                    .model
+                    .new_optional_ivar(lb, ub, self.chronicle.presence, label);
+                self.chronicle.cost = Some(IAtom { var, shift: i.shift });
             }
             _ => bail!("Cost must be an integer value."),
         };
diff --git a/planning/planners/src/encode.rs b/planning/planners/src/encode.rs
index f0c3f543..d81e55e8 100644
--- a/planning/planners/src/encode.rs
+++ b/planning/planners/src/encode.rs
@@ -146,6 +146,13 @@ pub fn instantiate(
         sub.add(v, fresh)?;
     }
 
+    if let Some(cost_template) = &template.chronicle.cost {
+        let label = lbl_of_new(cost_template.var.into(), &pb.model);
+        let (lb, ub) = pb.model.int_bounds(cost_template.var);
+        let cost_instance = pb.model.new_optional_ivar(lb, ub, prez_lit, label);
+        sub.add(cost_template.var.into(), cost_instance.into())?;
+    }
+
     template.instantiate(sub, origin)
 }
 
@@ -425,24 +432,15 @@ pub fn add_metric(pb: &FiniteProblem, model: &mut Model, metric: Metric) -> IAto
             plan_length.into()
         }
         Metric::ActionCosts => {
-            // retrieve the presence and cost of each chronicle
-            let mut costs = Vec::with_capacity(8);
-            for (ch_id, ch) in pb.chronicles.iter().enumerate() {
+            let mut action_costs = Vec::with_capacity(8);
+            for ch in pb.chronicles.iter() {
                 if let Some(cost) = &ch.chronicle.cost {
-                    costs.push((ch_id, ch.chronicle.presence, cost));
+                    let prez = ch.chronicle.presence;
+                    debug_assert!(prez == model.presence_literal(VarRef::from(cost.var)));
+                    action_costs.push(LinearTerm::variable(cost.var, prez));
+                    action_costs.push(LinearTerm::constant_int(cost.shift, prez));
                 }
             }
-
-            // for each action, create an optional variable that evaluate to the cost if the action is present and 0 otherwise
-            let action_costs: Vec<LinearTerm> = costs
-                .iter()
-                .map(|&(ch_id, p, cost)| {
-                    let bounds = model.domain_of(*cost);
-                    model
-                        .new_optional_ivar(bounds.0, bounds.1, p, Container::Instance(ch_id).var(VarType::Cost))
-                        .or_zero(p)
-                })
-                .collect();
             let action_costs = LinearSum::of(action_costs);
 
             // make the sum of the action costs equal a `plan_cost` variable.