Adding Support for Axioms and Derived Predicates #493
Conversation
|
Would appreciate this feature in my actual project, looking forward to it :D ! |
|
Having this in main would be great for the Fast Downward grounder (currently throwing an error when grounding introduces axioms because they cannot be represented in the grounded UP model). |
There was a problem hiding this comment.
@speckdavid Many thanks for this PR.
@roeger I know very little about derived predicates and I left only a couple of comments. Can you do a more detailed code review?
| from unified_planning.environment import get_environment, Environment | ||
|
|
||
|
|
||
| class Axiom(InstantaneousAction): |
There was a problem hiding this comment.
Why deriving from InstantaneousAction? This makes it possible to have Axioms passed to the add_action methods and causes some hacky code below. Why not a dedicated class?
There was a problem hiding this comment.
The idea was mainly to reuse the functionality and code of actions, not only when defining an action, but also for PDDL parsing. But I agree that we may have some side effects here. In general, I am open to creating a dedicated class not derived from InstantaneousAction, but I am a bit concerned that the PDDL parsing might become a bit more involved and we might get some redundant code.
There was a problem hiding this comment.
Can you have a look at the internal structure of the InstantaneousActiona and see what is that you really need? Maybe this could be a call to a small refactoring aimed at factorising what is needed to both instantaneous actions and axioms.
| @@ -75,6 +79,17 @@ def is_bool_type(self) -> bool: | |||
| return True | |||
|
|
|||
|
|
|||
| class _DerivedBoolType(Type): | |||
There was a problem hiding this comment.
Does this information needs to be in the typing system or we can keep a list/map/set of predicates that are derived in the Problem?
There was a problem hiding this comment.
In the beginning, I simply wanted to use the Boolean type, but I ran into some problems. For example, when a planning problem is created, an assertion checks whether all variables have an initial value. For derived predicates, however, we cannot set such a value.
I think your suggestion of having a list/set of variables that are derived would also work if we can access it at the appropriate places. It will definitely simplify the code as the typing system is not touched. I will check it.
|
I just stumbled over this as well: I had thought that the UP supports derived predicates already. Would be great to have this in the library! |
As part of the 3rd Sprint of the "Symbolic Search for Diverse Plans and Maximum Utility" project, we added support for Axioms + Derived Predicates to the UP. So that these can be represented internally as well as read from and written to PDDL.
We also created a notebook as a usage example: Axioms and Derived Predicates in the Unified Planning Library.
Best
David