AI-Planning / pddl

Unquestionable PDDL 3.1 parser
https://ai-planning.github.io/pddl/
MIT License
79 stars 26 forks source link

Subtype Incompatibility Error with Predicate Term Types #98

Open mgkwon opened 9 months ago

mgkwon commented 9 months ago

Subject of the issue

When a predicate is defined with a type and is called in other places such as an action precondition with a subtype as a term it fails due to the assert in: Predicate.__call__

I just started getting my hands on PDDL so I'm not sure if it is a good practice to utilize the hierarchy of the types. However, PDDL allows me so it should also be allowed. Please let me know what you think.

Your environment

Steps to reproduce

  1. Define a predicate, at, that expects two terms of type location.
  2. Define a variable, l_city, of type city.
  3. Attempt to use the at predicate with the term l_city.
p = Variable("p", ["person"])
l = Variable("l", ["location"])
l_city = Variable("l_city", ["city"])
at= Predicate("at", p, l)
...
work = Action(
    "work",
    parameters=[p, l_city],
    precondition=**at(p, l_city)**,
    effect=working(p),
)

Expected behaviour

The at predicate should allow the use of the l_city term because city is a subtype of location in the type hierarchy.

Actual behaviour

The library raises an error, indicating that the predicate terms' types are incorrect. It does not recognize that city is a subtype of location in the type hierarchy, resulting in an incorrect type check. AssertionError: Types of replacements is not correct.

marcofavorito commented 9 months ago

@mgkwon thank you very much for your interest in using pddl. I'm going to have a look at your issue and provide a fix asap!

fenjenahwe commented 4 months ago

Hi! I'm facing the same issue and was wondering if there are any updates regarding this. Is there any quick workaround to make subtypes compatible with predicates?

On a related note, I'm searching for the correct way to declare subtypes when defining the domain, if that is supported by the library. Any tips would be appreciated!

For example:

 requirements = [Requirements.STRIPS, Requirements.TYPING, Requirements.NEG_PRECONDITION]
    domain = Domain("dining-general",
    requirements=requirements,
    types=["objects location hypothesis - world", "agent furniture consumable - objects"],
    predicates=[...],
    actions=[...])

returns a type parsing error. Thanks!

haz commented 4 months ago

Hrmz...from the first example, it doesn't look like you have any sub-type information put in. Here's an example of how sub-typing can be done:

That said, it doesn't look like the Variable object is associated with the domain (similar with the predicate).

@marcofavorito should the Domain object (containing type hierarchy) be used in some way for the validation of parameters?

marcofavorito commented 4 months ago

Hi,

@fenjenahwe the types argument in the Domain class constructor is of type Dict[namelike, Optional[namelike], see https://github.com/AI-Planning/pddl/blob/8e3be082c0c2142288ffb14a9766f66bd9b1febf/pddl/core.py#L53C19-L53C68

Hence, in your example, you should write:

types={
    "objects": "world",
    "location": "world",
    "hypothesis": "world",
    "agent": "objects",
    "furniture": "objects",
    "consumable": "objects",
}

Regarding @mgkwon's issue, yes there should be a check in the Domain instantiation. Will try to provide a fix (this time for real :sweat_smile: ).

fenjenahwe commented 4 months ago

Thanks @haz While going through the code I stumbled upon the syntax you shared, but if I use it, and assuming I want A to be a subtype of B, and B to be a subtype of C, then it translates it simply into:

(define (domain dining-general)
    (:requirements :negative-preconditions :strips :typing)
    (:types A B C)

My understanding is that it should translate into:

(define (domain dining-general)
    (:requirements :negative-preconditions :strips :typing)
    (:types A - B - C)

the dash symbolizing a subtype relation (?) - unless I have misunderstood this (if so, would appreciate any resource I can look into to understand this).

For the second point, yes I believe this is the main issue - even when I manage to have a type hierarchy (cf. below), if a Predicate's parameter is supposed to have type C, then a parameter of type B will be rejected and return an error because it is understood as a separate/independent type (regardless of type hierarchy in the domain).

My current workaround (yikes, brace yourselves): I've edited the regex used to parse the types to include the space. This allows me to express the type hierarchy with dashes. https://github.com/AI-Planning/pddl/blob/8e3be082c0c2142288ffb14a9766f66bd9b1febf/pddl/custom_types.py#L30

Then disabled ( :grin: ) the lines that validate the types used in Predicates. https://github.com/AI-Planning/pddl/blob/8e3be082c0c2142288ffb14a9766f66bd9b1febf/pddl/logic/predicates.py#L73-L76

Doing this, I obtained a correct PDDL description of the domain/problem and was solvable.

fenjenahwe commented 4 months ago

Hi,

@fenjenahwe the types argument in the Domain class constructor is of type Dict[namelike, Optional[namelike], see https://github.com/AI-Planning/pddl/blob/8e3be082c0c2142288ffb14a9766f66bd9b1febf/pddl/core.py#L53C19-L53C68

Hence, in your example, you should write:

types={
    "objects": "world",
    "location": "world",
    "hypothesis": "world",
    "agent": "objects",
    "furniture": "objects",
    "consumable": "objects",
}

Regarding @mgkwon's issue, yes there should be a check in the Domain instantiation. Will try to provide a fix (this time for real 😅 ).

Thanks! If do write it this way, I get this:

(define (domain dining-general)
    (:requirements :negative-preconditions :strips :typing)
    (:types agent consumable furniture hypothesis location objects)

My understanding is that these are just types that are independent from each other? Or am I missing something..

marcofavorito commented 4 months ago

Hi, @fenjenahwe the types argument in the Domain class constructor is of type Dict[namelike, Optional[namelike], see https://github.com/AI-Planning/pddl/blob/8e3be082c0c2142288ffb14a9766f66bd9b1febf/pddl/core.py#L53C19-L53C68 Hence, in your example, you should write:

types={
    "objects": "world",
    "location": "world",
    "hypothesis": "world",
    "agent": "objects",
    "furniture": "objects",
    "consumable": "objects",
}

Regarding @mgkwon's issue, yes there should be a check in the Domain instantiation. Will try to provide a fix (this time for real 😅 ).

Thanks! If do write it this way, I get this:

(define (domain dining-general)
    (:requirements :negative-preconditions :strips :typing)
    (:types agent consumable furniture hypothesis location objects)

My understanding is that these are just types that are independent from each other? Or am I missing something..

This is another issue (different from the original one); I will make sure this is documented in another GH issue, and then addressed. Thank you very much for the report!

marcofavorito commented 4 months ago

Regarding @mgkwon's issue, I think a good solution would be to pass the type hierarchy definition inside the Predicate constructor, e.g.:

p = Variable("p", ["person"])
l = Variable("l", ["location"])
l_city = Variable("l_city", ["city"])
at= Predicate("at", p, l, types={...})

Basically, the argument types is of the same Python type as the argument types in the Domain constructor. This adds a bit more complexity in the initialization of Predicate's instances, but I guess it is a necessary evil to preserve the "composability", bottom-up style of our APIs (that is, first I initialize variables, then predicates, etc.).

The change is less straightforward than previously thought, and since a lot work has been done in this work-in-progress PR (https://github.com/AI-Planning/pddl/pull/84) I suggest to include the fix to this issue in that line of work.

haz commented 4 months ago

Should likely have a default for the types parameter so we don't need to specify it, ya?