From dd07f82086b1b5a12160076ef78b7d07c5e46797 Mon Sep 17 00:00:00 2001 From: Roland Godet Date: Thu, 5 Dec 2024 13:22:21 +0100 Subject: [PATCH 1/6] feat(planning): add goals in useless support --- .../analysis/detrimental_supports.rs | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/planning/planning/src/chronicles/analysis/detrimental_supports.rs b/planning/planning/src/chronicles/analysis/detrimental_supports.rs index 5268592a..2dcdf49e 100644 --- a/planning/planning/src/chronicles/analysis/detrimental_supports.rs +++ b/planning/planning/src/chronicles/analysis/detrimental_supports.rs @@ -100,14 +100,13 @@ pub fn find_useless_supports(pb: &Problem) -> HashSet { } } - let mut useful_values = HashSet::new(); // TODO: extend with goals - - for ch in &pb.templates { - for cond in &ch.chronicle.conditions { - let gval = value_of(&cond.state_var.fluent, &cond.state_var.args, cond.value); - useful_values.insert(gval); - } - } + let useful_values = pb + .chronicles + .iter() + .flat_map(|ch| ch.chronicle.conditions.iter()) + .chain(pb.templates.iter().flat_map(|t| t.chronicle.conditions.iter())) + .map(|c| value_of(&c.state_var.fluent, &c.state_var.args, c.value)) + .collect::>(); println!("Useful values:"); for v in &useful_values { @@ -294,7 +293,14 @@ fn gather_detrimental_supports( useful_values: &HashSet, detrimentals: &mut HashSet, ) { - let mut external_contributors = HashSet::new(); + let mut external_contributors = pb + .chronicles + .iter() + .flat_map(|ch| ch.chronicle.conditions.iter()) + .filter(|c| c.state_var.fluent.as_ref() == fluent) // Goals on the fluent + .map(|c| value_of(&c.state_var.fluent, &c.state_var.args, c.value)) + .collect::>(); + for ch in &pb.templates { let mut conds = HashMap::new(); for c in &ch.chronicle.conditions { From 4e58ed969dc556eacf533a1df981bad211a2f35b Mon Sep 17 00:00:00 2001 From: Roland Godet Date: Thu, 5 Dec 2024 13:23:05 +0100 Subject: [PATCH 2/6] style(planning): clean up --- .../analysis/detrimental_supports.rs | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/planning/planning/src/chronicles/analysis/detrimental_supports.rs b/planning/planning/src/chronicles/analysis/detrimental_supports.rs index 2dcdf49e..9aad3864 100644 --- a/planning/planning/src/chronicles/analysis/detrimental_supports.rs +++ b/planning/planning/src/chronicles/analysis/detrimental_supports.rs @@ -316,8 +316,6 @@ fn gather_detrimental_supports( let key = (&e.state_var, e.transition_start); if !conds.contains_key(&key) { return; // the fluent is not continuous - } else { - // conds.remove(&key).unwrap(); } } else { let EffectOp::Assign(value) = e.operation else { panic!() }; @@ -343,18 +341,6 @@ fn gather_detrimental_supports( println!(" - {}", c.format(pb)); } - // gather all values that are affected by non-optional chronicles (ie, non-templates) - let mut initial_values = HashSet::new(); - for ch in &pb.chronicles { - for e in &ch.chronicle.effects { - if e.state_var.fluent.as_ref() == fluent { - let EffectOp::Assign(value) = e.operation else { panic!() }; - let atom = value_of(&e.state_var.fluent, &e.state_var.args, value); - initial_values.insert(atom.value); - } - } - } - let conditions = find_conditions(fluent, pb); let transitions = find_transitions(fluent, pb); let supporters = |val: &GAtom| transitions.iter().filter(move |t| &t.post == val).collect_vec(); @@ -396,6 +382,18 @@ fn gather_detrimental_supports( // - always is the initial value // - is not useful in itself // - is the source/target of all transition to/from useful values + + // gather all values that are affected by non-optional chronicles (ie, non-templates) + let mut initial_values = HashSet::new(); + for ch in &pb.chronicles { + for e in &ch.chronicle.effects { + if e.state_var.fluent.as_ref() == fluent { + let EffectOp::Assign(value) = e.operation else { panic!() }; + let atom = value_of(&e.state_var.fluent, &e.state_var.args, value); + initial_values.insert(atom.value); + } + } + } let from_useful = transitions .iter() .filter(|t| external_contributors.contains(&t.pre)) @@ -413,7 +411,7 @@ fn gather_detrimental_supports( // true if the state variables always start from the initial value let transition_is_initial = initial_values.iter().all(|a| a == &transition_value.value); - if !external_contributors.contains(&transition_value) && transition_value_is_unique && transition_is_initial + if !external_contributors.contains(transition_value) && transition_value_is_unique && transition_is_initial { // we have our single transition value, // mark as detrimental all transitions to it. From d5ecb7f56cd7516ad68a52b0d27e8a7f03baecc9 Mon Sep 17 00:00:00 2001 From: Roland Godet Date: Thu, 5 Dec 2024 13:23:51 +0100 Subject: [PATCH 3/6] fix(planning): detrimental detection of non-single useful value --- .../analysis/detrimental_supports.rs | 75 +++++++++---------- 1 file changed, 36 insertions(+), 39 deletions(-) diff --git a/planning/planning/src/chronicles/analysis/detrimental_supports.rs b/planning/planning/src/chronicles/analysis/detrimental_supports.rs index 9aad3864..8468f01f 100644 --- a/planning/planning/src/chronicles/analysis/detrimental_supports.rs +++ b/planning/planning/src/chronicles/analysis/detrimental_supports.rs @@ -375,13 +375,11 @@ fn gather_detrimental_supports( } } } - } /* Deactivated as it may be incorrect in some lifted cases where unless checking that the return (supported) - transition ends up at the same value - else { - // detect pattern where the is a single transition value: - // - always is the initial value - // - is not useful in itself - // - is the source/target of all transition to/from useful values + } else { + // detect pattern where the is a single transition value: + // - always is the initial value + // - is not useful in itself + // - is the source/target of all transition to/from useful values // gather all values that are affected by non-optional chronicles (ie, non-templates) let mut initial_values = HashSet::new(); @@ -394,38 +392,37 @@ fn gather_detrimental_supports( } } } - let from_useful = transitions - .iter() - .filter(|t| external_contributors.contains(&t.pre)) - .collect_vec(); - let post_useful: HashSet<_> = from_useful.iter().map(|t| &t.post).collect(); - let to_useful = transitions - .iter() - .filter(|t| external_contributors.contains(&t.post)) - .collect_vec(); - let pre_useful: HashSet<_> = to_useful.iter().map(|t| &t.pre).collect(); - if post_useful.len() == 1 && post_useful == pre_useful { - let transition_value = post_useful.iter().next().copied().unwrap(); - // true if there is a unique tranisition value (ie, it does not correspond to a type that could have ultiple values) - let transition_value_is_unique = transition_value.value.is_constant(); - // true if the state variables always start from the initial value - let transition_is_initial = initial_values.iter().all(|a| a == &transition_value.value); + let from_useful = transitions + .iter() + .filter(|t| external_contributors.contains(&t.pre)) + .collect_vec(); + let post_useful: HashSet<_> = from_useful.iter().map(|t| &t.post).collect(); + let to_useful = transitions + .iter() + .filter(|t| external_contributors.contains(&t.post)) + .collect_vec(); + let pre_useful: HashSet<_> = to_useful.iter().map(|t| &t.pre).collect(); + if post_useful.len() == 1 && post_useful == pre_useful { + let transition_value = post_useful.iter().next().copied().unwrap(); + // true if there is a unique tranisition value (ie, it does not correspond to a type that could have ultiple values) + let transition_value_is_unique = transition_value.value.is_constant(); + // true if the state variables always start from the initial value + let transition_is_initial = initial_values.iter().all(|a| a == &transition_value.value); if !external_contributors.contains(transition_value) && transition_value_is_unique && transition_is_initial - { - // we have our single transition value, - // mark as detrimental all transitions to it. - // proof: any transition to it must be preceded by transition from it (with no other side effects) - for t1 in supporters(transition_value) { - for t2 in supported(transition_value) { - if t1.post == t2.pre { - // this is a transition from `transition_value -> useful_value -> transition_value` - detrimentals.insert(CausalSupport::transitive(t1, t2)); - } - } - } - } - - } - } */ + { + // we have our single transition value, + // mark as detrimental all transitions to it. + // proof: any transition to it must be preceded by transition from it (with no other side effects) + for t1 in supported(transition_value) { + for t2 in supporters(transition_value) { + if t1.post == t2.pre { + // this is a transition from `transition_value -> useful_value -> transition_value` + detrimentals.insert(CausalSupport::transitive(t1, t2)); + } + } + } + } + } + } } From 926b41150010e2af3071952b6b7002f0cfa4dd84 Mon Sep 17 00:00:00 2001 From: Roland Godet Date: Mon, 9 Dec 2024 06:29:27 +0100 Subject: [PATCH 4/6] docs(planning): Add clear comment about useful values in detrimental supports --- .../src/chronicles/analysis/detrimental_supports.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/planning/planning/src/chronicles/analysis/detrimental_supports.rs b/planning/planning/src/chronicles/analysis/detrimental_supports.rs index 8468f01f..b72b32c8 100644 --- a/planning/planning/src/chronicles/analysis/detrimental_supports.rs +++ b/planning/planning/src/chronicles/analysis/detrimental_supports.rs @@ -100,6 +100,8 @@ pub fn find_useless_supports(pb: &Problem) -> HashSet { } } + // Extract all values that are useful. + // A value is useful if it is a goal or a condition of a Chronicle template. let useful_values = pb .chronicles .iter() @@ -376,7 +378,7 @@ fn gather_detrimental_supports( } } } else { - // detect pattern where the is a single transition value: + // detect pattern where there is a single transition value: // - always is the initial value // - is not useful in itself // - is the source/target of all transition to/from useful values @@ -404,7 +406,7 @@ fn gather_detrimental_supports( let pre_useful: HashSet<_> = to_useful.iter().map(|t| &t.pre).collect(); if post_useful.len() == 1 && post_useful == pre_useful { let transition_value = post_useful.iter().next().copied().unwrap(); - // true if there is a unique tranisition value (ie, it does not correspond to a type that could have ultiple values) + // true if there is a unique transition value (ie, it does not correspond to a type that could have multiple values) let transition_value_is_unique = transition_value.value.is_constant(); // true if the state variables always start from the initial value let transition_is_initial = initial_values.iter().all(|a| a == &transition_value.value); @@ -417,7 +419,7 @@ fn gather_detrimental_supports( for t1 in supported(transition_value) { for t2 in supporters(transition_value) { if t1.post == t2.pre { - // this is a transition from `transition_value -> useful_value -> transition_value` + // this is a transition from `transition_value -> useful_value -> transition_value` detrimentals.insert(CausalSupport::transitive(t1, t2)); } } From 919fc566bfe6590be7d61487acf019b8b7750017 Mon Sep 17 00:00:00 2001 From: Roland Godet Date: Mon, 9 Dec 2024 08:16:23 +0100 Subject: [PATCH 5/6] refactor(planning): enable ARIES_USELESS_SUPPORTS by default --- planning/planners/src/encode/symmetry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planning/planners/src/encode/symmetry.rs b/planning/planners/src/encode/symmetry.rs index 46c5bd14..dd56a98c 100644 --- a/planning/planners/src/encode/symmetry.rs +++ b/planning/planners/src/encode/symmetry.rs @@ -16,7 +16,7 @@ use crate::Model; /// The value of this parameter is loaded from the environment variable `ARIES_LCP_SYMMETRY_BREAKING`. /// Possible values are `none` and `simple` (default). pub static SYMMETRY_BREAKING: EnvParam = EnvParam::new("ARIES_LCP_SYMMETRY_BREAKING", "psp"); -pub static USELESS_SUPPORTS: EnvParam = EnvParam::new("ARIES_USELESS_SUPPORTS", "false"); +pub static USELESS_SUPPORTS: EnvParam = EnvParam::new("ARIES_USELESS_SUPPORTS", "true"); pub static PSP_ABSTRACTION_HIERARCHY: EnvParam = EnvParam::new("ARIES_PSP_ABSTRACTION_HIERARCHY", "true"); /// The type of symmetry breaking to apply to problems. From 9412b4dc4f12a0ab416e7251bdd3e0df91fba38d Mon Sep 17 00:00:00 2001 From: Roland Godet Date: Mon, 9 Dec 2024 08:17:02 +0100 Subject: [PATCH 6/6] fix(planning): detrimental loop through transition value --- .../analysis/detrimental_supports.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/planning/planning/src/chronicles/analysis/detrimental_supports.rs b/planning/planning/src/chronicles/analysis/detrimental_supports.rs index b72b32c8..e9694331 100644 --- a/planning/planning/src/chronicles/analysis/detrimental_supports.rs +++ b/planning/planning/src/chronicles/analysis/detrimental_supports.rs @@ -413,13 +413,19 @@ fn gather_detrimental_supports( if !external_contributors.contains(transition_value) && transition_value_is_unique && transition_is_initial { - // we have our single transition value, - // mark as detrimental all transitions to it. - // proof: any transition to it must be preceded by transition from it (with no other side effects) - for t1 in supported(transition_value) { - for t2 in supporters(transition_value) { + // We have our single transition value. + // Mark all transition cycles `useful -> transition_value -> useful` as detrimental. + // Proof: There is not need to go through the transition value to reach + // the useful value again with no other intermediate value. + for t1 in supporters(transition_value) { + for t2 in supported(transition_value) { if t1.post == t2.pre { - // this is a transition from `transition_value -> useful_value -> transition_value` + println!( + " => Detrimental: {} -> {} -> {}", + t1.pre.format(pb), + t1.post.format(pb), + t2.post.format(pb) + ); detrimentals.insert(CausalSupport::transitive(t1, t2)); } }