-
Notifications
You must be signed in to change notification settings - Fork 88
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
Multiline predicate mangled by DomainReader but handled correctly by plansys2_pddl_parser
#286
Comments
Bckempa
added a commit
to Bckempa/isaac
that referenced
this issue
Dec 21, 2023
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!!! |
marinagmoreira
added a commit
to nasa/isaac
that referenced
this issue
Jan 5, 2024
* Add `survey_manager` subdirectory without layout for survey nodes Includes placeholder directories with hyperlinked `readme.md` files in the documentation hierarchy for the manger, planner, and bridge. Per discussion with @marinagmoreira `survey_manager` lives under `astrobee` to signify the code is intended for cross-compilation. * Improve linking to survey manager docs * Add files via upload Adding planning domain, problem, and xml of the atomic actions of the robotic agents * added files to the correct directory * adding soft constraints problem files * Attempt at a complete MVP domain model * Oops, remove redundant effect * Simplified collision checking predicate * In a working state * Add sample_output_plan.txt * Oops, remove obsolete commented-out code * Fixed stereo survey part of the model based on analysis of old stereo surveys by Marina - unfortunately causes additional planner flakiness * Update sample_output_plan.txt to reflect latest domain/problem * Now generate PDDL problem from higher-level problem specification * Tune panorama estimated duration * Add plan_interpreter.py. Minor cleanup in problem_generator.py * Simplify dynamic config just a bit * Add `survey_manager` subdirectory without layout for survey nodes Includes placeholder directories with hyperlinked `readme.md` files in the documentation hierarchy for the manger, planner, and bridge. Per discussion with @marinagmoreira `survey_manager` lives under `astrobee` to signify the code is intended for cross-compilation. * Improve linking to survey manager docs * Remove `survey_bridge` capability will be added to `astrobee` * Add traclabs plansys2 backport via submodule, thanks @ana-GT * Move sub-modules to `survey_manager` path * Upgrade behaviortree to V4 NOTE: If already installed, remove V3 before installing V4 * Cleanup unused, misplaced sub-modules, again. * Remove `survey_manager` package and organize `survey_planner` * Deprecate Ubuntu-16.04 (xenial) builds of Isaac to support Plansys2 - Remove Ubuntu-16.04 (xenial) CI builds - Update dockerfile ubuntu version defaults to Ubuntu-20.04 (focal) - Update apk build environment to Ubuntu-20.04 (focal) * Fix python formatting * Update to forks of traclabs backports * Add readline development files to setup for plansys2 terminal interface If you have already built your VM, run `sudo apt install libreadline-dev` * Removed outdated PDDL files per @trey0 #107 (comment) * Revert CI upgrades for APK builds * Plansys2 on Astrobee Noetic (#107) * Add `survey_manager` subdirectory without layout for survey nodes Includes placeholder directories with hyperlinked `readme.md` files in the documentation hierarchy for the manger, planner, and bridge. Per discussion with @marinagmoreira `survey_manager` lives under `astrobee` to signify the code is intended for cross-compilation. * Improve linking to survey manager docs * Add files via upload Adding planning domain, problem, and xml of the atomic actions of the robotic agents * added files to the correct directory * adding soft constraints problem files * Attempt at a complete MVP domain model * Oops, remove redundant effect * Simplified collision checking predicate * In a working state * Add sample_output_plan.txt * Oops, remove obsolete commented-out code * Fixed stereo survey part of the model based on analysis of old stereo surveys by Marina - unfortunately causes additional planner flakiness * Update sample_output_plan.txt to reflect latest domain/problem * Now generate PDDL problem from higher-level problem specification * Tune panorama estimated duration * Add plan_interpreter.py. Minor cleanup in problem_generator.py * Simplify dynamic config just a bit * Add `survey_manager` subdirectory without layout for survey nodes Includes placeholder directories with hyperlinked `readme.md` files in the documentation hierarchy for the manger, planner, and bridge. Per discussion with @marinagmoreira `survey_manager` lives under `astrobee` to signify the code is intended for cross-compilation. * Improve linking to survey manager docs * Remove `survey_bridge` capability will be added to `astrobee` * Add traclabs plansys2 backport via submodule, thanks @ana-GT * Move sub-modules to `survey_manager` path * Upgrade behaviortree to V4 NOTE: If already installed, remove V3 before installing V4 * Cleanup unused, misplaced sub-modules, again. * Remove `survey_manager` package and organize `survey_planner` * Deprecate Ubuntu-16.04 (xenial) builds of Isaac to support Plansys2 - Remove Ubuntu-16.04 (xenial) CI builds - Update dockerfile ubuntu version defaults to Ubuntu-20.04 (focal) - Update apk build environment to Ubuntu-20.04 (focal) * Fix python formatting * Update to forks of traclabs backports * Add readline development files to setup for plansys2 terminal interface If you have already built your VM, run `sudo apt install libreadline-dev` * Removed outdated PDDL files per @trey0 #107 (comment) * Revert CI upgrades for APK builds --------- Co-authored-by: Abiola Akanni <[email protected]> Co-authored-by: Trey Smith <[email protected]> * Update ci_pr.yml make survey manager PR's run CI * problem_generator.py: Simplified config, and runs with default args again * Removed run number argument from planner actions * Add support for PlanSys2 terminal output; remove num-orders from config * Refactor problem_generator.py to more cleanly support two output formats * plan_interpreter.py works with default arguments again * Update .isort.cfg using scripts/git/configure_isort_paths.sh * jem_survey_dynamic.yaml: Remove unused run parameter. * Fix legacy isort styling error in unrelated file so lint passes * Remove newlines in PDDL domain predicates to work around [1] [1] PlanSys2/ros2_planning_system#286 * Setup planner stack for action tests * Move submodules to avoid nested catkin packages * Added let-other-robot-reach constraint so we can solve the full problem with POPF/OPTIC * Fix pylint and mypy complaints * Add `popf` package and remove vendored solver in favor of built binary * Update plansys2 backport to fix domain validation and parameters * Make predicates on one line. * Update vendored libraries for cross compilation * making submodules their own folder for organization * updating submodule + fix bug in cmake * only check isort files that were modified * src dir doesn't exist anymore and usually not in include * fix gitmodules link * fixing PR CI --------- Co-authored-by: Abiola Akanni <[email protected]> Co-authored-by: Trey Smith <[email protected]> Co-authored-by: Marina Moreira <[email protected]> Co-authored-by: Brian Coltin <[email protected]> Co-authored-by: Marina Moreira <[email protected]>
The easiest way would be to leverage the PDDL parser within the DomainReader |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Consider the case of a PDDL domain definition with a predicate defined with newlines between the arguments, like this:
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: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: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?
The text was updated successfully, but these errors were encountered: