-
Notifications
You must be signed in to change notification settings - Fork 158
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
PDDLSimplePlanParser is not able to parse Contingent-FF #255
Comments
After checking with more detail I understand that the FFPlannerInterface transforms the output of the FF planner to POPF plan format. So, the PDDLSimplePlanParser is able to parse the results of FF planner. This transformation to POPF plan format is what I named as ROSPlan defined format. Therefore, the problem of parsing the Contingent-FF is not related to PDDLSimplePlanParser but to the FFPlannerInterface, and it is related to #254. This should be solved by transforming the Contingent-FF plan file to POPF plan format in FFPlannerInterface. |
Working on this issue, see: https://github.com/dgerod/ROSPlan/tree/feature/Add-ContingentFF-planner |
Pull request #256. |
|
in particular, a contingent plan is a tree structure and after the ROSPlan converts it to a ROS message action by parsing interface, is there any objectives to create a contingent plan. |
@bilal9876, sorry but I cannot get your point. |
I mean that a contingency plan is a plan where the effect of any action may result in 'x' state (AND branch) or in 'y' state (OR branch). When the contingency plan is converted to temporal plan, (what is implemented in ROSPlan framework), the temporal plan considers only one branch (AND branch) as the effect of the action. So, after parsing the plan and then dispatching the plan, when action fail or the result of the action is not the same as the predicted one, the ROSPlan framework will replan again from the scratch and will not consider the another branch (OR branch) of the previously contingent plan. is this true or not ? |
Hello Bilal, You are correct, the esterel/simple plan parsing assumes non-contingent plans. For parsing contingent plans you should use the PNP parser, which will generate a petr-net-plan for dispatch - you can find that here: About separating the planner interface and plan parsing nodes - this is how it should be. If they were one node, this couples the plan representation to the planner, which is not always desired. For example, I might want to generate plans with POPF and represent them as ESTEREL, PNP, or some other format. In addition, this allows to publish the plan as string to be shown to the user, or connected to some other component (not plan parsing and dispatch). Michael |
@m312z , one question about this issue. This issue is related to a missed functionality not with the question you are answering. I opened a PR to fixing it some time ago but it is still not merged, so is it correct to close this issue? Are you closing this one because this one already exists? Thanks. |
The parsed plan from Contingent-FF is not being generated by PDDLSimplePlanParser. And it is normal because the parser is expected a syntax as POPF planner:
0.000: (goto_place b1) [0.001]
While Contingent-FF output to be parser is:
0||0 --- GOTO_PLACE C1 --- SON: 1||-1
And I think that this is not working also with FF, its syntax is:
0: PICKUP_OBJECT C1 V1
In the documentation is written that planner interface works with FF, Metric-FF and Contingent-FF among others. So, my understanding is that all nodes of ROSPlan work with Contingent-FF what is not happening.
Therefore, in my opinion this is a bug. Could you confirm that my understanding is correct? Thanks.
Finally, I would like to talk about the design of "planning_interface" and "parse_plan nodes". Parsing a planner output is hard coupled to the planner itself, so why these two functionalities are split in two different nodes? In my opinion they should be done by only one node that is the "planner_interface", and it should generate a ROSPlan defined format of actions.
In addition, I think that having the "planner_command" parameter for "planner_interface" is not a good idea because it is also hard coupled with implementation of the planner (e.g. POPFPlannerInterface). The only information that is not couple to the planner is the timeout and the directory path where the planner binary is stored, so these should be passed to the node as parameters.
The text was updated successfully, but these errors were encountered: