Feature/multi robot planner rebase #17

GoldenZephyr · 2025-10-03T13:53:14Z

@JY-HIM4U I updated your branch to be up-to-date with a more recent version of Omniplanner. I tried to run pddl_example_multirobot_FD_ominiplanner.py locally but it failed trying to import 'MultiRobotPddlDomain' from 'dsg_pddl.pddl_grounding'. I don't see MultiRobotPddlDomain in pddl_grounding.py, but maybe I'm missing something?

GoldenZephyr

Looking good. I made a couple of specific notes

GoldenZephyr · 2025-10-03T13:54:03Z

...nner/examples/Sample_from_Jake/From_Aaron/b45_clip_final_connected_rooms_and_labelspace.json

Let's not add any of the Sample_from_Jake directory to the repo

GoldenZephyr · 2025-10-03T13:59:05Z

omniplanner/src/dsg_pddl/dsg_pddl_grounding_Multirobot_Better.py

+logger = logging.getLogger(__name__)
+
+
+def explicit_edges_from_layer(


Can we reuse any of these functions from the single-robot case? I think the edge construction should be exactly the same, and maybe a couple of other functions?

GoldenZephyr · 2025-10-03T14:00:10Z

omniplanner/src/dsg_pddl/dsg_pddl_grounding_Multirobot_Better.py

+
+    # Filter goal string to only include available objects
+    available_object_names = [o.symbol for o in object_symbols]
+    print(f"DEBUG: Available objects: {available_object_names}")


If you want to add debug prints, you should use logger.debug, because once we are running from inside a ROS node it will hide all of the print outputs.

GoldenZephyr · 2025-10-03T14:01:23Z

omniplanner/src/dsg_pddl/dsg_pddl_grounding_Multirobot_Better.py

+    print("length of all_objects: ", len(all_objects))
+    print("length of all_places: ", len(all_places))
+    # Apply optional limits to keep the problem manageable
+    max_places = int(os.environ.get("MR_MAX_PLACES", "20"))


Now that you have a smaller scene graph to test on we can probably remove these, but if you still want limits like this then we should have them passed in as arguments and not read directly from the environment.

GoldenZephyr · 2025-10-03T14:04:23Z

omniplanner/src/dsg_pddl/dsg_pddl_grounding_Multirobot_Better.py

+    init_facts += [fmt_fact(f) for f in object_at_facts]
+    init_facts += [fmt_fact(f) for f in suspicious_facts]
+
+    problem_str = """


Take a look at PddlProblem in pddl_grounding.py -- We have a class where you can set domain, objects, initial_facts, goal, and turn optimizing on or off, and it will dump to the correct PDDL problem string

GoldenZephyr · 2025-10-03T14:21:44Z

Oh, also you should set up pre-commit to do code formatting / linting for you.
pipx install pre-commit , and then pre-commit run --all-files, or pre-commit install if you want to have it run automatically every time you commit.

GoldenZephyr · 2025-10-10T20:45:23Z

@JY-HIM4U I just pushed the compile_plan implementation that should allow us to turn the multi-robot plan into multiple single-robot plans. It should be runnable with the same example script (although you might need to change back some paths that I changed). There's still some other cleanup to do next week, but I think the biggest other thing is just the integration of the ROS plugin side. Let me know if you run into any issues with that.

GoldenZephyr

Just a couple of small cleanup requests.

GoldenZephyr · 2025-10-17T21:52:30Z

omniplanner/src/dsg_pddl/dsg_pddl_grounding_Multirobot_Better.py

+    return f"(and {' '.join(safe_goals)})"
+
+
+# def generate_multirobot_inspection_pddl(


I'd like to either delete this chunk of commented code, or uncomment it and document what it's for

GoldenZephyr · 2025-10-17T21:52:57Z

omniplanner/src/dsg_pddl/dsg_pddl_grounding_Multirobot_Better.py

+    normalize_symbols(symbols)
+
+    symbols_of_interest = symbols
+    # print("robot_states: ", robot_states)


Let's either uncomment or delete these prints / logger messages

GoldenZephyr · 2025-10-17T21:55:09Z

omniplanner_ros/src/omniplanner_ros/multirobot_ros.py

+
+@overload
+@dispatch
+def compile_plan(adaptor, plan_frame: str, plan: PddlPlan):


I'm pretty sure we don't need any of the compile_plan implementations here. Did you need to add these or were they leftover from copying the original file? As far as I know, we can remove them.

GoldenZephyr · 2025-10-17T21:58:18Z

omniplanner/src/omniplanner/omniplanner.py



 def full_planning_pipeline(plan_request: PlanRequest, map_context: Any, feedback=None):
+    import dsg_pddl.dsg_pddl_grounding_Multirobot_Better  # noqa: F401


I think we should be able to get by without these now, at least when running in ROS. Could you check if you can still generate the plan in ROS without these two lines?

[python-1] File "/home/jaeyoun-choi/colcon_ws/build/omniplanner/src/omniplanner/omniplanner.py", line 270, in full_planning_pipeline [python-1] grounded_problem = ground_problem( [python-1] ^^^^^^^^^^^^^^^ [python-1] File "/usr/local/lib/python3.12/dist-packages/plum/function.py", line 383, in __call__ [python-1] return _convert(method(*args, **kw_args), return_type) [python-1] ^^^^^^^^^^^^^^^^^^^^^^^^ [python-1] File "/home/jaeyoun-choi/colcon_ws/build/omniplanner/src/dsg_pddl/dsg_pddl_grounding.py", line 516, in ground_problem [python-1] raise NotImplementedError( [python-1] NotImplementedError: I don't know how to ground a domain of type region-object-rearrangement-domain-multirobot-fd! [python-1] The following exception was never retrieved: cannot use Destroyable because destruction was requested [ERROR] [python-1]: process has died [pid 1566420, exit code 1, cmd '/home/jaeyoun-choi/environments/dcist/spark_env/bin/python /home/jaeyoun-choi/colcon_ws/install/omniplanner_ros/lib/omniplanner_ros/omniplanner_node --ros-args -r __node:=omniplanner_node -r __ns:=/hilbert --params-file /tmp/launch_params_tgi5owmo --params-file /tmp/launch_params_1bza027h -r ~/node_status:=ros_system_monitor/node_diagnostic_collector -r ~/dsg_in:=hydra/backend/dsg -r ~/llm_response:=rviz2_node/llm_response'].

Above error happens without above import lines. I moved those lines to the top of the code, but it isn't work with other error.

Ok I'll take a look at that

I didn't get a chance to actually run this on my side today, but could you try moving these two import lines to multirobot_ros.py ? I think that should lead to the right imports, although I didn't have time to double check.

GoldenZephyr

Just a little bit of cleanup left, see the comments. There's also a small bug I found. This line that does some postprocessing to generate the geometric path for the robot to follow after a PDDL plan is found doesn't work properly in the multirobot case (https://github.com/MIT-SPARK/Omniplanner/blob/main/omniplanner/src/dsg_pddl/dsg_pddl_planning.py#L88). Applying the following patch fixes it for testing, although it would break single-robot planning.

diff --git a/omniplanner/src/dsg_pddl/dsg_pddl_planning.py b/omniplanner/src/dsg_pddl/dsg_pddl_planning.py
index fdc8a66..3760a9b 100644
--- a/omniplanner/src/dsg_pddl/dsg_pddl_planning.py
+++ b/omniplanner/src/dsg_pddl/dsg_pddl_planning.py
@@ -87,8 +87,8 @@ def make_plan(grounded_problem: GroundedPddlProblem, map_context: Any) -> PddlPl
         match p[0]:
             case "goto-poi":
                 path = layer_planner.get_external_path(
-                    grounded_problem.symbols[p[1]].position,
                     grounded_problem.symbols[p[2]].position,
+                    grounded_problem.symbols[p[3]].position,
                 )
                 parameterized_plan.append(path)
                 last_pose = path[-1]

I haven't had a chance to think through a solution, although it would most likely require either adding some information to the GroundedPddlProblem that we get as input so that we can decide to process as a single or multi robot plan, or maybe we don't build this "parameterized plan" here at all and instead we do it during compile_plan where we are already reasoning about the single- and multi- robot versions of these actions.

GoldenZephyr · 2025-10-21T03:11:48Z

omniplanner_ros/src/omniplanner_ros/multirobot_ros.py

+    return pt
+
+
+# @overload


We can get rid of this commented code. (Also I think we don't need ensure_3d here anymore either.

omniplanner/src/dsg_pddl/dsg_pddl_grounding_multirobot.py

GoldenZephyr · 2025-10-21T03:19:34Z

omniplanner/src/omniplanner/omniplanner.py



 def full_planning_pipeline(plan_request: PlanRequest, map_context: Any, feedback=None):
+    import dsg_pddl.dsg_pddl_grounding_Multirobot_Better  # noqa: F401


I didn't get a chance to actually run this on my side today, but could you try moving these two import lines to multirobot_ros.py ? I think that should lead to the right imports, although I didn't have time to double check.

GoldenZephyr · 2025-10-21T13:31:13Z

I think I fixed the single-robot vs. multi-robot indexing. I also deleted the commented out code. So could you

take a final look through the review comments (I think maybe all that's left is a file renaming, and trying the other file import location)
Confirm on your end that the planning works through ROS

And then I think we can merge

GoldenZephyr · 2025-10-21T13:34:13Z

Ok I think I fixed the imports, seems to work with the new location.

JY-HIM4U · 2025-10-21T14:08:37Z

Thanks for your feedback!

I changed the import location and it works! Also changing file name to 'dsg_pddl_grounding_multirobot.py' is great.

I changed the file name and pushed it.

GoldenZephyr

Looks good!

Restore unnecessary modification Changing folder Name chore: ignore local scenegraph JSON data Modify multi-robot pddl domain so that visited- predicates doesn't need to be defined with specific robots. Define MultiRobotPddlDomain for ground_problem for multi-robot. Now,omniplanner.py is also restored to the original version except the import dsg_pddl.dsg_pddl_planning formatted Add MultiRobotPddlDomain to pddl_grounding.py, Remove unnecessary functions and case for goto-object-domain in dsg_pddl_grounding_Multirobot_Better.py, Fix format using pre-commit implement compile_plan for multirobot planning ROS for multirobotplan jaeyoun local Changing ROS for Multi-robot planner Cleanup by following Aaron's feedback I think that planning should work for single-robot and multi-robot domains Multi-robot domains currently must have a name that contains the string Fix import location Change name to dsg_pddl_grounding_multirobot.py Change name to dsg_pddl_grounding_multirobot at multirobot_ros.py Remove unused code Modify generate_place_containment.py for place-in-region issue Modify generate_place_containment.py for place-in-region issue Debugging for multi-robot action

GoldenZephyr commented Oct 3, 2025

View reviewed changes

JY-HIM4U force-pushed the feature/multi_robot_planner_rebase branch from 6deb31b to 2cc2e72 Compare October 17, 2025 01:23

GoldenZephyr commented Oct 17, 2025

View reviewed changes

GoldenZephyr commented Oct 21, 2025

View reviewed changes

GoldenZephyr mentioned this pull request Oct 21, 2025

Modify omniplanner for Multi-robot planning. #16

Closed

GoldenZephyr commented Oct 23, 2025

View reviewed changes

GoldenZephyr force-pushed the feature/multi_robot_planner_rebase branch from 1188e38 to 75b4d2d Compare October 23, 2025 16:36

GoldenZephyr merged commit 7cdbdb1 into main Oct 23, 2025

		logger = logging.getLogger(__name__)


		def explicit_edges_from_layer(

		return f"(and {' '.join(safe_goals)})"


		# def generate_multirobot_inspection_pddl(



		def full_planning_pipeline(plan_request: PlanRequest, map_context: Any, feedback=None):
		import dsg_pddl.dsg_pddl_grounding_Multirobot_Better # noqa: F401

Feature/multi robot planner rebase #17

Feature/multi robot planner rebase #17

Uh oh!

Conversation

GoldenZephyr commented Oct 3, 2025

Uh oh!

GoldenZephyr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GoldenZephyr commented Oct 3, 2025

Uh oh!

GoldenZephyr commented Oct 10, 2025

Uh oh!

GoldenZephyr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GoldenZephyr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GoldenZephyr commented Oct 21, 2025

Uh oh!

GoldenZephyr commented Oct 21, 2025

Uh oh!

JY-HIM4U commented Oct 21, 2025

Uh oh!

GoldenZephyr left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants