From bc743ff93227797601b8840172e0db911a7e7c82 Mon Sep 17 00:00:00 2001
From: Gabriele Roeger <gabriele.roeger@unibas.ch>
Date: Thu, 1 Feb 2024 14:30:55 +0100
Subject: [PATCH] [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.