Merge pull request #119 from AI-Planning/and-condeff-fix

An effect part of the condeff should be AndEffect and not And.

pddl/_validation.py

@@ -23,8 +23,8 @@
 from pddl.exceptions import PDDLValidationError
 from pddl.helpers.base import check, ensure, ensure_set, find_cycle
 from pddl.logic import Predicate
-from pddl.logic.base import BinaryOp, QuantifiedCondition, UnaryOp
-from pddl.logic.effects import AndEffect, Forall, When
+from pddl.logic.base import And, BinaryOp, QuantifiedCondition, UnaryOp
+from pddl.logic.effects import Forall, When
 from pddl.logic.functions import (
     BinaryFunction,
     FunctionExpression,
@@ -285,7 +285,7 @@ def _(self, formula: QuantifiedCondition) -> None:
         self.check_type(formula.condition)
 
     @check_type.register
-    def _(self, effect: AndEffect) -> None:
+    def _(self, effect: And) -> None:
         """Check types annotations of a PDDL and-effect."""
         self.check_type(effect.operands)
 

pddl/logic/effects.py

@@ -12,57 +12,14 @@
 
 """This modules implements PDDL effects."""
 import functools
-from typing import AbstractSet, Collection, Generic, Optional, Sequence, TypeVar, Union
+from typing import AbstractSet, Collection, Optional, Union
 
 from pddl.helpers.base import _typed_parameters, ensure_set
 from pddl.helpers.cache_hash import cache_hash
 from pddl.logic import Variable
-from pddl.logic.base import Atomic, Formula, Not, OneOf
+from pddl.logic.base import And, Atomic, Formula, Not, OneOf
 from pddl.parser.symbols import Symbols
 
-EffectType = TypeVar("EffectType")
-
-
-@cache_hash
-@functools.total_ordering
-class AndEffect(Generic[EffectType]):
-    """Conjunction of effects."""
-
-    def __init__(self, *operands: EffectType):
-        """
-        Initialize a conjunction of (conditional) effects.
-
-        :param operands: the operands.
-        """
-        self._operands = list(operands)
-
-    @property
-    def operands(self) -> Sequence[EffectType]:
-        """Get the operands."""
-        return tuple(self._operands)
-
-    def __str__(self) -> str:
-        """Get the string representation."""
-        return f"({Symbols.AND.value} {' '.join(map(str, self.operands))})"
-
-    def __repr__(self) -> str:
-        """Get an unambiguous string representation."""
-        return f"{type(self).__name__}({repr(self._operands)})"
-
-    def __eq__(self, other):
-        """Compare with another object."""
-        return isinstance(other, type(self)) and self.operands == other.operands
-
-    def __lt__(self, other) -> bool:
-        """Compare with another object."""
-        if isinstance(other, AndEffect):
-            return tuple(self.operands) < tuple(other.operands)
-        return super().__lt__(other)  # type: ignore
-
-    def __hash__(self) -> int:
-        """Compute the hash of the object."""
-        return hash((type(self), self.operands))
-
 
 @cache_hash
 @functools.total_ordering
@@ -167,5 +124,5 @@ def __lt__(self, other):
 
 PEffect = Union[Atomic, Not]
 CEffect = Union[Forall, When, OneOf, "PEffect"]
-Effect = Union[AndEffect["CEffect"], CEffect]
-CondEffect = Union[AndEffect["PEffect"], "PEffect"]
+Effect = Union[And, CEffect]
+CondEffect = Union[And, "PEffect"]

pddl/parser/domain.py

@@ -11,7 +11,7 @@
 #
 
 """Implementation of the PDDL domain parser."""
-from typing import Any, Dict, Optional, Set, Tuple
+from typing import Any, Dict, List, Optional, Sequence, Set, Tuple
 
 from lark import Lark, ParseError, Transformer
 
@@ -21,7 +21,7 @@
 from pddl.exceptions import PDDLMissingRequirementError, PDDLParsingError
 from pddl.helpers.base import assert_, call_parser
 from pddl.logic.base import And, ExistsCondition, ForallCondition, Imply, Not, OneOf, Or
-from pddl.logic.effects import AndEffect, Forall, When
+from pddl.logic.effects import Forall, When
 from pddl.logic.functions import Assign, Decrease, Divide
 from pddl.logic.functions import EqualTo as FunctionEqualTo
 from pddl.logic.functions import (
@@ -50,7 +50,7 @@
 class DomainTransformer(Transformer[Any, Domain]):
     """Domain Transformer."""
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, *args, **kwargs) -> None:
         """Initialize the domain transformer."""
         super().__init__(*args, **kwargs)
 
@@ -258,7 +258,7 @@ def effect(self, args):
         if len(args) == 1:
             return args[0]
         if args[1] == Symbols.AND.value:
-            return AndEffect(*args[2:-1])
+            return And(*args[2:-1])
         raise ValueError("case not recognized")
 
     def c_effect(self, args):
@@ -337,10 +337,12 @@ def constant(self, args):
             raise ParseError(f"Constant '{args[0]}' not defined.")
         return constant
 
-    def _formula_skeleton(self, args):
+    def _formula_skeleton(self, args) -> Sequence[Variable]:
         """Process the '_formula_skeleton' rule."""
-        variable_data: Dict[str, Set[str]] = args[2]
-        variables = [Variable(var_name, tags) for var_name, tags in variable_data]
+        variable_data: Tuple[Tuple[str, Set[str]], ...] = args[2]
+        variables: List[Variable] = [
+            Variable(var_name, tags) for var_name, tags in variable_data
+        ]
         return variables
 
     def atomic_formula_skeleton(self, args):

pddl/parser/problem.py

@@ -44,7 +44,7 @@
 class ProblemTransformer(Transformer[Any, Problem]):
     """Problem Transformer."""
 
-    def __init__(self):
+    def __init__(self) -> None:
         """Initialize the problem transformer."""
         super().__init__()
 

tests/fixtures/code_objects/blocksworld_fond.py

@@ -17,7 +17,7 @@
 from pddl.core import Domain, Problem
 from pddl.logic import Constant
 from pddl.logic.base import And, OneOf
-from pddl.logic.effects import AndEffect, When
+from pddl.logic.effects import When
 from pddl.logic.helpers import constants, variables
 from pddl.logic.predicates import EqualTo, Predicate
 from pddl.requirements import Requirements
@@ -52,15 +52,15 @@ def blocksworld_fond_domain():
         & ~EqualTo(x, table)
     )
     put_on_effect = OneOf(
-        AndEffect(
+        And(
             on(x, y),
             ~on(x, z),
             When(~EqualTo(z, table), clear(z)),
             When(~EqualTo(y, table), ~clear(y)),
         ),
-        AndEffect(
+        And(
             on(x, table),
-            When(~EqualTo(z, table), ~on(x, z) & clear(z)),
+            When(~EqualTo(z, table), And(~on(x, z), clear(z))),
             When(~EqualTo(y, table), ~clear(y)),
         ),
     )

tests/fixtures/code_objects/blocksworld_ipc08.py

@@ -16,7 +16,6 @@
 from pddl.action import Action
 from pddl.core import Domain, Problem
 from pddl.logic.base import And, OneOf
-from pddl.logic.effects import AndEffect
 from pddl.logic.helpers import constants, variables
 from pddl.logic.predicates import EqualTo, Predicate
 from pddl.requirements import Requirements
@@ -48,8 +47,8 @@ def blocksworld_domain():
     pick_up_parameters = [b1, b2]
     pick_up_precondition = ~EqualTo(b1, b2) & emptyhand & clear(b1) & on(b1, b2)
     pick_up_effect = OneOf(
-        AndEffect(holding(b1), clear(b2), ~emptyhand, ~clear(b1), ~on(b1, b2)),
-        AndEffect(clear(b2), on_table(b1), ~on(b1, b2)),
+        And(holding(b1), clear(b2), ~emptyhand, ~clear(b1), ~on(b1, b2)),
+        And(clear(b2), on_table(b1), ~on(b1, b2)),
     )
     pick_up = Action(
         pick_up_name, pick_up_parameters, pick_up_precondition, pick_up_effect
@@ -59,9 +58,7 @@ def blocksworld_domain():
     pick_up_from_table_name = "pick-up-from-table"
     pick_up_from_table_parameters = [b]
     pick_up_from_table_precondition = emptyhand & clear(b) & on_table(b)
-    pick_up_from_table_effect = OneOf(
-        AndEffect(), AndEffect(holding(b), ~emptyhand, ~on_table(b))
-    )
+    pick_up_from_table_effect = OneOf(And(), And(holding(b), ~emptyhand, ~on_table(b)))
     pick_up_from_table = Action(
         pick_up_from_table_name,
         pick_up_from_table_parameters,
@@ -74,8 +71,8 @@ def blocksworld_domain():
     put_on_block_parameters = [b1, b2]
     put_on_block_precondition = holding(b1) & clear(b2)
     put_on_block_effect = OneOf(
-        AndEffect(on(b1, b2), emptyhand, clear(b1), ~holding(b1), ~clear(b2)),
-        AndEffect(on_table(b1), emptyhand, clear(b1), ~holding(b1)),
+        And(on(b1, b2), emptyhand, clear(b1), ~holding(b1), ~clear(b2)),
+        And(on_table(b1), emptyhand, clear(b1), ~holding(b1)),
     )
     put_on_block = Action(
         put_on_block_name,
@@ -88,7 +85,7 @@ def blocksworld_domain():
     put_down_name = "put-down"
     put_down_parameters = [b]
     put_down_precondition = holding(b)
-    put_down_effect = AndEffect(on_table(b), emptyhand, clear(b), ~holding(b))
+    put_down_effect = And(on_table(b), emptyhand, clear(b), ~holding(b))
     put_down = Action(
         put_down_name, put_down_parameters, put_down_precondition, put_down_effect
     )
@@ -97,7 +94,7 @@ def blocksworld_domain():
     pick_tower_parameters = [b1, b2, b3]
     pick_tower_precondition = emptyhand & on(b1, b2) & on(b2, b3)
     pick_tower_effect = OneOf(
-        AndEffect(), AndEffect(holding(b2), clear(b3), ~emptyhand, ~on(b2, b3))
+        And(), And(holding(b2), clear(b3), ~emptyhand, ~on(b2, b3))
     )
     pick_tower = Action(
         pick_tower_name,
@@ -111,8 +108,8 @@ def blocksworld_domain():
     put_tower_on_block_parameters = [b1, b2, b3]
     put_tower_on_block_precondition = holding(b2) & on(b1, b2) & clear(b3)
     put_tower_on_block_effect = OneOf(
-        AndEffect(on(b2, b3), emptyhand, ~holding(b2), ~clear(b3)),
-        AndEffect(on_table(b2), emptyhand, ~holding(b2)),
+        And(on(b2, b3), emptyhand, ~holding(b2), ~clear(b3)),
+        And(on_table(b2), emptyhand, ~holding(b2)),
     )
     put_tower_on_block = Action(
         put_tower_on_block_name,
@@ -125,7 +122,7 @@ def blocksworld_domain():
     put_tower_down_name = "put-tower-down"
     put_tower_down_parameters = [b1, b2]
     put_tower_down_precondition = holding(b2) & on(b1, b2)
-    put_tower_down_effect = AndEffect(on_table(b2), emptyhand, ~holding(b2))
+    put_tower_down_effect = And(on_table(b2), emptyhand, ~holding(b2))
     put_tower_down = Action(
         put_tower_down_name,
         put_tower_down_parameters,

tests/fixtures/code_objects/triangle_tireworld.py

@@ -16,7 +16,6 @@
 from pddl.action import Action
 from pddl.core import Domain, Problem
 from pddl.logic.base import And, OneOf
-from pddl.logic.effects import AndEffect
 from pddl.logic.helpers import constants, variables
 from pddl.logic.predicates import Predicate
 from pddl.requirements import Requirements
@@ -46,10 +45,10 @@ def triangle_tireworld_domain():
     move_car_name = "move-car"
     move_car_parameters = [from_, to]
     move_car_precondition = vehicleat(from_) & road(from_, to) & not_flattire
-    move_car_effect = AndEffect(
+    move_car_effect = And(
         OneOf(
-            AndEffect(vehicleat(to), ~vehicleat(from_)),
-            AndEffect(vehicleat(to), ~vehicleat(from_), ~not_flattire),
+            And(vehicleat(to), ~vehicleat(from_)),
+            And(vehicleat(to), ~vehicleat(from_), ~not_flattire),
         )
     )
     move_car = Action(
@@ -60,7 +59,7 @@ def triangle_tireworld_domain():
     changetire_name = "changetire"
     changetire_parameters = [loc]
     changetire_precondition = spare_in(loc) & vehicleat(loc)
-    changetire_effect = AndEffect(~spare_in(loc), not_flattire)
+    changetire_effect = And(~spare_in(loc), not_flattire)
     changetire = Action(
         changetire_name,
         changetire_parameters,

tests/test_formatter.py

@@ -21,8 +21,7 @@
 from pddl.core import Domain, Problem
 from pddl.formatter import domain_to_string, problem_to_string
 from pddl.logic import Constant, Variable, constants
-from pddl.logic.base import ForallCondition
-from pddl.logic.effects import AndEffect
+from pddl.logic.base import And, ForallCondition
 from pddl.logic.functions import (
     EqualTo,
     GreaterEqualThan,
@@ -119,7 +118,7 @@ def test_numerical_hello_world_domain_formatter():
         "say-hello-world",
         parameters=[neighbor],
         precondition=LesserEqualThan(hello_counter, NumericValue(3)),
-        effect=AndEffect(Increase(hello_counter, NumericValue(1))),
+        effect=And(Increase(hello_counter, NumericValue(1))),
     )
 
     domain = Domain(
@@ -137,7 +136,7 @@ def test_numerical_hello_world_domain_formatter():
             "    (:action say-hello-world",
             "        :parameters (?neighbor)",
             "        :precondition (<= (hello_counter ?neighbor) 3)",
-            "        :effect (and (increase (hello_counter ?neighbor) 1))",
+            "        :effect (increase (hello_counter ?neighbor) 1)",
             "    )",
             ")",
         )