Merge pull request #130 from morxa/fix-function-variable-types

Use correctly typed variables when parsing functions

Author: haz

.github/workflows/test.yml

@@ -29,7 +29,7 @@ jobs:
         pyver=`echo ${{ matrix.python-version }} | tr -d "."`
         tox -e py${{ matrix.python-version }}
     - name: Upload coverage to Codecov
-      uses: codecov/codecov-action@v1
+      uses: codecov/codecov-action@v5
       with:
         token: ${{ secrets.CODECOV_TOKEN }}
         file: ./coverage.xml

pddl/parser/domain.py

@@ -306,27 +306,24 @@ def num_effect(self, args):
         else:
             raise PDDLParsingError(f"Unrecognized assign operator: {args[1]}")
 
+    def _constant_or_variable(self, t):
+        """Get the constant or variable with the given name."""
+        # Case where the term is a free variable (bug) or comes from a parent quantifier
+        if not isinstance(t, Constant) and t not in self._current_parameters_by_name:
+            return Variable(str(t), {})
+        return t if isinstance(t, Constant) else self._current_parameters_by_name[t]
+
     def atomic_formula_term(self, args):
         """Process the 'atomic_formula_term' rule."""
-
-        def constant_or_variable(t):
-            # Case where the term is a free variable (bug) or comes from a parent quantifier
-            if (
-                not isinstance(t, Constant)
-                and t not in self._current_parameters_by_name
-            ):
-                return Variable(str(t), {})
-            return t if isinstance(t, Constant) else self._current_parameters_by_name[t]
-
         if args[1] == Symbols.EQUAL.value:
             if not bool({Requirements.EQUALITY} & self._extended_requirements):
                 raise PDDLMissingRequirementError(Requirements.EQUALITY)
-            left = constant_or_variable(args[2])
-            right = constant_or_variable(args[3])
+            left = self._constant_or_variable(args[2])
+            right = self._constant_or_variable(args[3])
             return EqualTo(left, right)
         else:
             predicate_name = args[1]
-            terms = list(map(constant_or_variable, args[2:-1]))
+            terms = list(map(self._constant_or_variable, args[2:-1]))
             return Predicate(predicate_name, *terms)
 
     def constant(self, args):
@@ -387,7 +384,7 @@ def f_head(self, args):
         if len(args) == 1:
             return NumericFunction(args[0])
         function_name = args[1]
-        variables = [Variable(x, {}) for x in args[2:-1]]
+        variables = list(map(self._constant_or_variable, args[2:-1]))
         return NumericFunction(function_name, *variables)
 
     def typed_list_name(self, args) -> Dict[name, Optional[name]]:

tests/test_parser/test_domain.py

@@ -16,6 +16,9 @@
 import lark
 import pytest
 
+from pddl.logic.functions import BinaryFunction, Increase, NumericFunction
+from pddl.logic.predicates import Predicate
+from pddl.logic.terms import Variable
 from pddl.parser.domain import DomainParser
 from pddl.parser.symbols import Symbols
 from tests.conftest import TEXT_SYMBOLS
@@ -317,3 +320,59 @@ def test_check_action_costs_requirement_with_total_cost() -> None:
         match=r"action costs requirement is not specified, but the total-cost function is specified.",
     ):
         DomainParser()(domain_str)
+
+
+def test_variable_types_in_strips_action_definition() -> None:
+    """Check typing for predicate variables in action preconditions and effects."""
+    domain_str = dedent(
+        """
+    (define (domain test)
+        (:requirements :typing)
+        (:types t1 t2)
+        (:predicates (p ?x - t1 ?y - t2))
+        (:action a
+            :parameters (?x - t1 ?y - t2)
+            :precondition (p ?x ?y)
+            :effect (p ?x ?y)
+        )
+    )
+    """
+    )
+    domain = DomainParser()(domain_str)
+    action = next(iter(domain.actions))
+    x = Variable("x", {"t1"})
+    y = Variable("y", {"t2"})
+    assert action.parameters == (x, y)
+    assert isinstance(action.precondition, Predicate)
+    assert action.precondition.terms == (x, y)
+    assert isinstance(action.effect, Predicate)
+    assert action.effect.terms == (x, y)
+
+
+def test_variable_types_in_numeric_action_definition() -> None:
+    """Check typing for function variables in action preconditions and effects."""
+    domain_str = dedent(
+        """
+    (define (domain test)
+        (:requirements :typing :numeric-fluents)
+        (:types t1 t2)
+        (:functions (f ?x - t1 ?y - t2))
+        (:action a
+            :parameters (?x - t1 ?y - t2)
+            :precondition (<= 1 (f ?x ?y))
+            :effect (increase (f ?x ?y) 1)
+        )
+    )
+    """
+    )
+    domain = DomainParser()(domain_str)
+    action = next(iter(domain.actions))
+    x = Variable("x", {"t1"})
+    y = Variable("y", {"t2"})
+    assert action.parameters == (x, y)
+    assert isinstance(action.precondition, BinaryFunction)
+    assert isinstance(action.precondition.operands[1], NumericFunction)
+    assert action.precondition.operands[1].terms == (x, y)
+    assert isinstance(action.effect, Increase)
+    assert isinstance(action.effect.operands[0], NumericFunction)
+    assert action.effect.operands[0].terms == (x, y)