Implement BehaviourTrees for forall and imply

[eshan-savla]

Changes:

This pull request introduces several changes to the plansys2_domain_expert, plansys2_problem_expert, plansys2_pddl_parser and plansys2_executor packages, primarily to enhance the handling of durative actions by introducing new PDDL functionality using forall and imply and corresponding tests.

Parser updates:

• Corrected parsing method to match intended functionality of imply key in Imply.cpp.
• Implemented getTree methods for both forall and imply. Respective changes in Forall.cpp and Imply.cpp.
• Added optional instances_map field to pure virtual method in Condition.h and respective derived classes to allow forall to supplement action params with instances from typed lists.

Domain expert updates:

• To support BT creation for forall and its child trees, instances are now provided to the getDurativeAction method to supply the necessary instances to the forall getTree method.
• Updated pure virtual method DomainExpertInterface::getDurativeAction and its overrides to optionally accept a plansys2_msgs::msg::Param vector of instances.
• DomainExpert::getDurativeAction converts this vector into a map of format instance-type : instances and supplies this to all three precondition types. Effects are excluded at this point, as POPF support for forall in effects is limited. Extending support for forall in effects is simply a matter of providing these instances to the AtStart and AtEnd effects.

Updates to executor nodes:

• All instances are requested from the problem expert when a request to execute is recieved.
• All calls to the getDurativeAction method of the domain client are supplied with these instances in ExecutorNode.cpp and BTBuilder.cpp.

Updates to problem expert:

• New cases for forall and imply are are added to the evaluate method in Utils.cpp.
• As imply can only be used in preconditions, no logic for effect application is implemented for its case.

New PDDL functionality and tests:

• A new test case is added to bt_node_test.cpp within the executor package with the appropriate pddl domain to test the new functionalities implemented.

[fmrico]

Hi @eshan-savla

Thanks for your work!!

My main concern here is that it breaks the modular design of PlanSys2 if we fill everything with parameters to pass instances back and forth. Maybe at some point it is necessary, but, for example, the domain expert, whose function is to describe and verify what has to do with the domain through the queries that are made to it, should not know anything about instances. That is what the problem expert is for. Nor should the bt_builder know anything about instances. Its mission is to build a behavior tree from a plan. Its nodes will be the ones to verify the conditions.

As far as I remember, it should be enough to check the corresponding plansys2_msgs::msg::Tree. @Rezenders has been working in similar duties. What do you think is the best approach?

[eshan-savla]

Hello to both of you,

Considering those points, another option would be to use placeholders for the forall parameters which are then filled in by the problem expert during evaluation.

The only thing I can't seem to figure out is creating the necessary amount of children for forall in the parser without knowing how many instances exist. It definitely would be nice to get a second opinion on solving this 😊

[roveri-marco]

Hi all, I agree with the position of @fmrico. We shall maintain modularity. Moreover, the temporal planners POPF and TFD as far as I know do not support forall constructs! Thus, it is not clear why adding them. Notice that, adding those construct to POPF is far from being trivial (as far I know). Moreover, we should consider using a new version of POPF that fixes some issues the currently available one has. But we need to merge the fixes since @fmrico applied some modifications to popf to support some checks not supported by the other popf version. Notice also, that the forall has an impact also on the construction of the BT since to properly capture the semantics of the temporal plan, constraints shall be imposed between nodes not only relying on the temporal aspects. My two cents!

[eshan-savla]

Hi @roveri-marco,
My experience has been different with both planners. During my usage of both planners, I have seen that both of them support the forall construct. TFD supports it to a higher degree than POPF, allowing it to be used in conditions and effects both. I have however only tested this on the TFD hosted on the planning as a service website. POPF support is limited, allowing its use in conditions and effects in certain cases, as stated here. I have found a useful case for it during my work on my masters thesis and thought that adding it as a feature would only be a benefit.

I don't disagree however, that my implementation of forall definitely doesn't help with the modularity and look forward to hearing suggestions on how to address that issue and also the second point you mentioned about the BTs inability to capture temporal plan semantics.

I really appreciate all of you guys weighing on this and offering your inputs. I hope I haven't caused too much extra work for you guys and would love to help correct any issues in my PR. 😄

[roveri-marco]

Hi @eshan-savla

Not sure the popf on the website is the same used in plansys2 (I have examples that work on the website and do not work on the one in plansys2). Moreover, type hierarchies are wrongly handled in both the domain and problem expert although supported by popf and tfd. TFD since based on decision epochs is not complete!
From my point of view we shall restrict to the features common to the different planners, since otherwise we may face problems. Given this, I'm not against extending, but we shall have a better support of the "normal" features of PDDL before adding new features supported by few planners and only in particular cases, and inform the user if a feature is not available in the planner.

[fmrico]

I disagree a bit here. We should allow PlanSys2 to do everything that PDDL allows, and let the user who uses one planner or another limit it based on what their planner supports. We'd have to see how to do it...

[roveri-marco]

@fmrico Ok, but then we need to properly report the user of unsupported features by the planners! Notice that PDDL is a rich language and the support by planners is questionable and also planner dependent! My experience with many planners is that users if they see fancy feature tend to use them, but then they fight with the planners that do not support them. I'm facing this problem every year with my students!