Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subtype Incompatibility Error with Predicate Term Types #98

Open
mgkwon opened this issue Nov 20, 2023 · 9 comments
Open

Subtype Incompatibility Error with Predicate Term Types #98

mgkwon opened this issue Nov 20, 2023 · 9 comments

Comments

@mgkwon
Copy link

mgkwon commented Nov 20, 2023

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

  • OS: Ubuntu Focal
  • Python version: 3.8.10
  • Package Version 0.4.0

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
Copy link
Member

@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
Copy link

fenjenahwe commented May 8, 2024

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
Copy link
Contributor

haz commented May 8, 2024

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
Copy link
Member

marcofavorito commented May 8, 2024

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 😅 ).

@fenjenahwe
Copy link

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.

REGEX = re.compile("[A-Za-z][-_A-Za-z0-9]*")

Then disabled ( 😁 ) the lines that validate the types used in Predicates.

assert_(
all(t1.type_tags == t2.type_tags for t1, t2 in zip(self.terms, terms)),
"Types of replacements is not correct.",
)

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

@fenjenahwe
Copy link

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
Copy link
Member

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
Copy link
Member

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 (#84) I suggest to include the fix to this issue in that line of work.

@haz
Copy link
Contributor

haz commented May 10, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants