From 917a6c41e7de89d51ffe770dc62be47af9988a81 Mon Sep 17 00:00:00 2001 From: Gabriele Roeger Date: Wed, 31 Jan 2024 12:52:37 +0100 Subject: [PATCH 1/4] [issue913] fix instantiation bug with uninitialized numeric expressions --- src/translate/normalize.py | 24 ++++++++++++++++++++++-- src/translate/pddl_to_prolog.py | 7 +++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/translate/normalize.py b/src/translate/normalize.py index 375dc67e9c..e0c061956d 100755 --- a/src/translate/normalize.py +++ b/src/translate/normalize.py @@ -3,6 +3,7 @@ import copy import pddl +import pddl_to_prolog class ConditionProxy: def clone_owner(self): @@ -23,7 +24,19 @@ def delete_owner(self, task): def build_rules(self, rules): action = self.owner rule_head = get_action_predicate(action) - rule_body = condition_to_rule_body(action.parameters, self.condition) + + # if the action cost is based on a primitive numeric expression, + # we need to require that it has a value defined in the initial state. + # We hand it over to condition_to_rule_body to include this in the rule + # body. + pne = None + if (isinstance(action.cost, pddl.Increase) and + isinstance(action.cost.expression, + pddl.PrimitiveNumericExpression)): + pne = action.cost.expression + + rule_body = condition_to_rule_body(action.parameters, self.condition, + pne) rules.append((rule_body, rule_head)) def get_type_map(self): return self.owner.type_map @@ -366,10 +379,13 @@ def build_exploration_rules(task): proxy.build_rules(result) return result -def condition_to_rule_body(parameters, condition): +def condition_to_rule_body(parameters, condition, numeric_expression=None): result = [] + # require parameters to be of the right type for par in parameters: result.append(par.get_atom()) + + # require the given condition if not isinstance(condition, pddl.Truth): if isinstance(condition, pddl.ExistentialCondition): for par in condition.parameters: @@ -388,6 +404,10 @@ def condition_to_rule_body(parameters, condition): assert isinstance(part, pddl.Literal), "Condition not normalized: %r" % part if not part.negated: result.append(part) + + # require the numeric expression (from the action cost) to be defined. + if numeric_expression is not None: + result.append(pddl_to_prolog.get_definition_fluent(numeric_expression)) return result if __name__ == "__main__": diff --git a/src/translate/pddl_to_prolog.py b/src/translate/pddl_to_prolog.py index fee70f7c3b..8bbb9d57e7 100755 --- a/src/translate/pddl_to_prolog.py +++ b/src/translate/pddl_to_prolog.py @@ -142,6 +142,9 @@ def __str__(self): cond_str = ", ".join(map(str, self.conditions)) return "%s :- %s." % (self.effect, cond_str) +def get_definition_fluent(pne: pddl.PrimitiveNumericExpression): + return pddl.Atom(f"@def-{pne.symbol}", pne.args) + def translate_typed_object(prog, obj, type_dict): supertypes = type_dict[obj.type_name].supertype_names for type_name in [obj.type_name] + supertypes: @@ -155,6 +158,10 @@ def translate_facts(prog, task): assert isinstance(fact, pddl.Atom) or isinstance(fact, pddl.Assign) if isinstance(fact, pddl.Atom): prog.add_fact(fact) + else: + # Add a fact to indicate that the primitive numeric expression in + # fact.fluent has been defined. + prog.add_fact(get_definition_fluent(fact.fluent)) def translate(task): # Note: The function requires that the task has been normalized. From 7dc2a92e78472244147cbe6be5d54bde0016a5aa Mon Sep 17 00:00:00 2001 From: Gabi Roeger Date: Thu, 1 Feb 2024 13:58:22 +0100 Subject: [PATCH 2/4] Improve comments Co-authored-by: remochristen --- src/translate/normalize.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/translate/normalize.py b/src/translate/normalize.py index e0c061956d..82c721315e 100755 --- a/src/translate/normalize.py +++ b/src/translate/normalize.py @@ -25,7 +25,7 @@ def build_rules(self, rules): action = self.owner rule_head = get_action_predicate(action) - # if the action cost is based on a primitive numeric expression, + # If the action cost is based on a primitive numeric expression, # we need to require that it has a value defined in the initial state. # We hand it over to condition_to_rule_body to include this in the rule # body. @@ -381,11 +381,11 @@ def build_exploration_rules(task): def condition_to_rule_body(parameters, condition, numeric_expression=None): result = [] - # require parameters to be of the right type + # Require parameters to be of the right type. for par in parameters: result.append(par.get_atom()) - # require the given condition + # Require the given condition. if not isinstance(condition, pddl.Truth): if isinstance(condition, pddl.ExistentialCondition): for par in condition.parameters: @@ -405,7 +405,7 @@ def condition_to_rule_body(parameters, condition, numeric_expression=None): if not part.negated: result.append(part) - # require the numeric expression (from the action cost) to be defined. + # Require the primitive numeric expression (from the action cost) to be defined. if numeric_expression is not None: result.append(pddl_to_prolog.get_definition_fluent(numeric_expression)) return result From bc743ff93227797601b8840172e0db911a7e7c82 Mon Sep 17 00:00:00 2001 From: Gabriele Roeger Date: Thu, 1 Feb 2024 14:30:55 +0100 Subject: [PATCH 3/4] [issue913] address comments from code review --- src/translate/normalize.py | 29 ++++++++++++++++++++++------- src/translate/pddl_to_prolog.py | 5 +---- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/translate/normalize.py b/src/translate/normalize.py index 82c721315e..723da142b5 100755 --- a/src/translate/normalize.py +++ b/src/translate/normalize.py @@ -1,9 +1,9 @@ #! /usr/bin/env python3 import copy +from typing import Sequence import pddl -import pddl_to_prolog class ConditionProxy: def clone_owner(self): @@ -130,6 +130,9 @@ def get_axiom_predicate(axiom): variables += [par.name for par in axiom.condition.parameters] return pddl.Atom(name, variables) +def get_pne_definition_predicate(pne: pddl.PrimitiveNumericExpression): + return pddl.Atom(f"@def-{pne.symbol}", pne.args) + def all_conditions(task): for action in task.actions: yield PreconditionProxy(action) @@ -379,13 +382,24 @@ def build_exploration_rules(task): proxy.build_rules(result) return result -def condition_to_rule_body(parameters, condition, numeric_expression=None): +def condition_to_rule_body(parameters: Sequence[pddl.TypedObject], + condition: pddl.conditions.Condition, + pne: pddl.PrimitiveNumericExpression = None): + """The rule body requires that + - all parameters (including existentially quantified variables in the + condition) are instantiated with objecst of the right type, + - all positive atoms in the condition (which must be normalized) are + true in the Prolog model, and + - the primitive numeric expression (from the action cost) has a defined + value (in the initial state).""" result = [] - # Require parameters to be of the right type. + # Require parameters to be instantiated with objects of the right type. for par in parameters: result.append(par.get_atom()) - # Require the given condition. + # Require each positive literals in the condition to be reached and + # existentially quantified variables of the condition to be instantiated + # with objects of the right type. if not isinstance(condition, pddl.Truth): if isinstance(condition, pddl.ExistentialCondition): for par in condition.parameters: @@ -405,9 +419,10 @@ def condition_to_rule_body(parameters, condition, numeric_expression=None): if not part.negated: result.append(part) - # Require the primitive numeric expression (from the action cost) to be defined. - if numeric_expression is not None: - result.append(pddl_to_prolog.get_definition_fluent(numeric_expression)) + # Require the primitive numeric expression (from the action cost) to be + # defined. + if pne is not None: + result.append(get_pne_definition_predicate(pne)) return result if __name__ == "__main__": diff --git a/src/translate/pddl_to_prolog.py b/src/translate/pddl_to_prolog.py index 8bbb9d57e7..7950451bec 100755 --- a/src/translate/pddl_to_prolog.py +++ b/src/translate/pddl_to_prolog.py @@ -142,9 +142,6 @@ def __str__(self): cond_str = ", ".join(map(str, self.conditions)) return "%s :- %s." % (self.effect, cond_str) -def get_definition_fluent(pne: pddl.PrimitiveNumericExpression): - return pddl.Atom(f"@def-{pne.symbol}", pne.args) - def translate_typed_object(prog, obj, type_dict): supertypes = type_dict[obj.type_name].supertype_names for type_name in [obj.type_name] + supertypes: @@ -161,7 +158,7 @@ def translate_facts(prog, task): else: # Add a fact to indicate that the primitive numeric expression in # fact.fluent has been defined. - prog.add_fact(get_definition_fluent(fact.fluent)) + prog.add_fact(normalize.get_pne_definition_predicate(fact.fluent)) def translate(task): # Note: The function requires that the task has been normalized. From 598e4c9dced2c9a7ccc01141a7c5e530c0f4549a Mon Sep 17 00:00:00 2001 From: Gabi Roeger Date: Thu, 1 Feb 2024 15:31:25 +0100 Subject: [PATCH 4/4] Fix typo in comment. Co-authored-by: remochristen --- src/translate/normalize.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/translate/normalize.py b/src/translate/normalize.py index 723da142b5..4b5df01dda 100755 --- a/src/translate/normalize.py +++ b/src/translate/normalize.py @@ -397,7 +397,7 @@ def condition_to_rule_body(parameters: Sequence[pddl.TypedObject], for par in parameters: result.append(par.get_atom()) - # Require each positive literals in the condition to be reached and + # Require each positive literal in the condition to be reached and # existentially quantified variables of the condition to be instantiated # with objects of the right type. if not isinstance(condition, pddl.Truth):