PlanSys2 / ros2_planning_system

This repo contains a PDDL-based planning system for ROS2.
Apache License 2.0
384 stars 86 forks source link

Multiline predicate mangled by DomainReader but handled correctly by `plansys2_pddl_parser` #286

Open Bckempa opened 8 months ago

Bckempa commented 8 months ago

Consider the case of a PDDL domain definition with a predicate defined with newlines between the arguments, like this:

(:predicates
    (foo ?foo - foobar)
    (bar
         ?foo - foobar
         ?bar - foobar
         ?baz - foobar
    )
)

This is treated differently be different components of PlanSys2. Running ros2 run plansys2_pddl_parser parser correct groups the arguments and returns a domain like this:

( :predicates
        ( foo ?foo - foobar )
        ( bar ?foo - foobar ?bar - foobar ?baz - foobar )
)

However, when read by the DomainExpert as a model file, DomainReader::get_predicates stores the predicates block (sans comments and newlines) before DomainReader::get_joint_domain reconstructs the predicate block assuming one full predicate per newline which are then passed through a std::set as deduplication with the side-effect of reordering the lines, leading to the following nonsensical block being passed to the POPFPlanSolver::isDomainValid check which syntax errors:

(:predicates
         ?bar - foobar
         ?baz - foobar
         ?foo - foobar
    (bar
    (foo ?foo - foobar)
    )
)

I checked the documentation and could not find a requirement that PDDL predicates not contain newlines and the referenced paper for the PDDL definition has BNF syntax for the language that also doesn't appear to settle this issue.

I'm happy to contribute a PR, but which behavior is correct - should the parser reject the split predicate, or should the DomainReader parsing handle this case?

fmrico commented 8 months ago

Hi @Bckempa

This is clearly a bug, and the correct behavior for the DomainReader is to handle this case.

It would be great to have a PR from you. If you prefer, I can try fixing it, but you may want to contribute. Let me know if I can help you :)

Thanks!!!

roveri-marco commented 5 months ago

The easiest way would be to leverage the PDDL parser within the DomainReader