-
Notifications
You must be signed in to change notification settings - Fork 5
Migrate EmbodiAgent #82
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 38 out of 39 changed files in this pull request and generated 51 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if agent_config.get("TaskAgent") is not None: | ||
| self.task_agent = TaskAgent( | ||
| task_llm, | ||
| **agent_config["Agent"], | ||
| **agent_config["TaskAgent"], | ||
| task_name=task_name, | ||
| ) | ||
| self.code_agent = CodeAgent( | ||
| code_llm, | ||
| **agent_config["Agent"], | ||
| **agent_config.get("CodeAgent"), | ||
| task_name=task_name, | ||
| ) | ||
| self.validation_agent = ValidationAgent( | ||
| validation_llm, | ||
| task_name=task_name, | ||
| task_description=self.code_agent.prompt_kwargs.get("task_prompt")[ | ||
| "content" | ||
| ], | ||
| basic_background=self.code_agent.prompt_kwargs.get("basic_background")[ | ||
| "content" | ||
| ], | ||
| atom_actions=self.code_agent.prompt_kwargs.get("atom_actions")["content"], | ||
| ) |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The agent initialization in _init_agents is missing error handling when agent_config does not contain expected keys. If agent_config doesn't have 'Agent', 'TaskAgent', or 'CodeAgent' keys, this will raise a KeyError. Consider adding validation or default values to handle missing configuration gracefully.
| class InjectKwargs(ast.NodeTransformer): | ||
| def visit_Call(self, node): | ||
| self.generic_visit(node) | ||
| # Inject **kwargs if not present | ||
| has_kwargs = any( | ||
| kw.arg is None | ||
| and isinstance(kw.value, ast.Name) | ||
| and kw.value.id == "kwargs" | ||
| for kw in node.keywords | ||
| ) | ||
| if not has_kwargs: | ||
| node.keywords.append( | ||
| ast.keyword( | ||
| arg=None, value=ast.Name(id="kwargs", ctx=ast.Load()) | ||
| ) | ||
| ) | ||
| return node | ||
|
|
||
| # Transform AST to inject kwargs | ||
| tree = InjectKwargs().visit(tree) | ||
| ast.fix_missing_locations(tree) |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The AST injection of kwargs into function calls (InjectKwargs transformer) modifies all function calls globally within the code, not just drive() calls. This could have unintended side effects if the generated code calls other functions that don't expect **kwargs. Consider making this more targeted to only inject kwargs into specific function calls like drive() and atomic action functions.
| def extract_drive_calls(code_str: str) -> list[str]: | ||
| tree = ast.parse(code_str) | ||
| lines = code_str.splitlines() | ||
|
|
||
| drive_blocks = [] | ||
|
|
||
| for node in tree.body: | ||
| # Match: drive(...) | ||
| if ( | ||
| isinstance(node, ast.Expr) | ||
| and isinstance(node.value, ast.Call) | ||
| and isinstance(node.value.func, ast.Name) | ||
| and node.value.func.id == "drive" | ||
| ): | ||
| # AST line numbers are 1-based | ||
| start = node.lineno - 1 | ||
| end = node.end_lineno | ||
| block = "\n".join(lines[start:end]) | ||
| drive_blocks.append(block) | ||
|
|
||
| return drive_blocks |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function extract_drive_calls uses ast.parse on user-provided code_str without any validation or error handling. If the code string is malformed or contains syntax errors, this will raise an exception. Add try-except block to handle parsing errors gracefully.
| def drive( | ||
| left_arm_action=None, | ||
| right_arm_action=None, | ||
| env=None, | ||
| **kwargs, | ||
| ): | ||
|
|
||
| if left_arm_action is not None and right_arm_action is not None: | ||
| len_left = len(left_arm_action) | ||
| len_right = len(right_arm_action) | ||
|
|
||
| if len_left < len_right: | ||
| diff = len_right - len_left | ||
| padding = np.repeat(left_arm_action[-1:], diff, axis=0) | ||
| left_arm_action = np.concatenate([left_arm_action, padding], axis=0) | ||
| elif len_right < len_left: | ||
| diff = len_left - len_right | ||
| padding = np.repeat(right_arm_action[-1:], diff, axis=0) | ||
| right_arm_action = np.concatenate([right_arm_action, padding], axis=0) | ||
|
|
||
| left_arm_index = env.left_arm_joints + env.left_eef_joints | ||
| right_arm_index = env.right_arm_joints + env.right_eef_joints | ||
| actions = np.zeros((len(right_arm_action), len(env.init_qpos))) | ||
| actions[:, left_arm_index] = left_arm_action | ||
| actions[:, right_arm_index] = right_arm_action | ||
|
|
||
| elif left_arm_action is None and right_arm_action is not None: | ||
| left_arm_index = env.left_arm_joints + env.left_eef_joints | ||
| right_arm_index = env.right_arm_joints + env.right_eef_joints | ||
| left_arm_action = finalize_actions( | ||
| env.left_arm_current_qpos, env.left_arm_current_gripper_state | ||
| ) | ||
| left_arm_action = np.repeat( | ||
| left_arm_action[None, :], len(right_arm_action), axis=0 | ||
| ) | ||
|
|
||
| actions = np.zeros( | ||
| (len(right_arm_action), len(env.robot.get_qpos().squeeze(0))), | ||
| dtype=np.float32, | ||
| ) | ||
| actions[:, left_arm_index] = left_arm_action | ||
| actions[:, right_arm_index] = right_arm_action | ||
|
|
||
| elif right_arm_action is None and left_arm_action is not None: | ||
| left_arm_index = env.left_arm_joints + env.left_eef_joints | ||
| right_arm_index = env.right_arm_joints + env.right_eef_joints | ||
| right_arm_action = finalize_actions( | ||
| env.right_arm_current_qpos, env.right_arm_current_gripper_state | ||
| ) | ||
| right_arm_action = np.repeat( | ||
| right_arm_action[None, :], len(left_arm_action), axis=0 | ||
| ) | ||
|
|
||
| actions = np.zeros( | ||
| (len(left_arm_action), len(env.robot.get_qpos().squeeze(0))), | ||
| dtype=np.float32, | ||
| ) | ||
| actions[:, left_arm_index] = left_arm_action | ||
| actions[:, right_arm_index] = right_arm_action | ||
|
|
||
| else: | ||
| log_error("At least one arm action should be provided.") | ||
|
|
||
| actions = torch.from_numpy(actions).to(dtype=torch.float32).unsqueeze(1) | ||
| actions = list(actions.unbind(dim=0)) | ||
| for i in tqdm(range(len(actions))): | ||
| action = actions[i] | ||
| obs, reward, terminated, truncated, info = env.step(action) | ||
| return actions |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The drive() function modifies environment state via env.step() in a loop without any error handling. If step() raises an exception (e.g., due to invalid action), the loop will terminate abruptly. Additionally, the return value 'actions' is a list of tensors but the function documentation doesn't specify this return type. Add error handling and document the return type.
| response = view_selection_llm.invoke(messages).content.strip() | ||
|
|
||
| if response not in img_dirs: | ||
| raise ValueError(f"Invalid camera selection from LLM: {response}") | ||
|
|
||
| return response |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validation_llm.invoke() call in select_best_view_dir doesn't have any error handling. If the LLM call fails or times out, this will raise an unhandled exception. Additionally, if the LLM returns an unexpected value (not in img_dirs), a ValueError is raised, but this might be too strict - consider logging and falling back to a default camera instead.
| import time | ||
| import cv2 | ||
| import glob | ||
| import json |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'json' is not used.
| import json |
| # ---------------------------------------------------------------------------- | ||
|
|
||
| import torch | ||
| import numpy as np |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'np' is not used.
| import numpy as np |
| ChatPromptTemplate, | ||
| HumanMessagePromptTemplate, | ||
| ) | ||
| from embodichain.utils.utility import encode_image, encode_image_from_path |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'encode_image_from_path' is not used.
| from embodichain.utils.utility import encode_image, encode_image_from_path | |
| from embodichain.utils.utility import encode_image |
embodichain/toolkits/toolkits.py
Outdated
|
|
||
| from abc import ABCMeta, abstractmethod | ||
| import os | ||
| import cv2 |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'cv2' is not used.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@yangchen73 I've opened a new pull request, #86, to work on those changes. Once the pull request is ready, I'll request review from you. |
| @@ -0,0 +1,31 @@ | |||
| { "TaskAgent": { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change the folder into pour_water_agent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. I'v renamed this folder.
embodichain/agents/README.md
Outdated
| @@ -0,0 +1,97 @@ | |||
| # EmbodiAgent System | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add agent related docs into docs/features folder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. I'll rewrite the docs and move it to docs/features.
embodichain/agents/README.md
Outdated
|
|
||
| ```bash | ||
| # Set environment variables | ||
| export AZURE_OPENAI_ENDPOINT="[https://your-endpoint.openai.azure.com/](https://your-endpoint.openai.azure.com/)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add docs to show how to change different LLM/VLM API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I'll add it into the docs which discribe agent system.
| @@ -0,0 +1,5 @@ | |||
| Task: | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move prompt file to configs/gym/agent/{specific_task} would be better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good suggustion.
| @@ -0,0 +1,136 @@ | |||
| ### Atom Functions for Robot Arm Control | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
database folder has been removed. Put these reusable prompts into agent folder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved it
| return "{}_{}_qpos".format(scope, tag) | ||
|
|
||
|
|
||
| def get_control_part(env, agent_uid): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These teo functions seems useless. Please removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
embodichain/data/enum.py
Outdated
| return dofs > 10 | ||
|
|
||
|
|
||
| class HandQposNormalizer: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove useless objects in this file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: yangchen73 <115123709+yangchen73@users.noreply.github.com>
| ) | ||
|
|
||
| # Run main function | ||
| main(args, env, gym_config) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from .run_env import main instead of rewrite it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed.
| code_file_path, kwargs, _ = self.generate_code_for_actions( | ||
| regenerate=regenerate | ||
| ) | ||
| action_list = self.code_agent.act(code_file_path, **kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to define self.action_length = len(action_list) for truncate detection inside the environment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'v defined it.
Description
This PR migrates EmbodiAgent, a LLM-based agent system for autonomous robot task execution in EmbodiChain. The system enables robots to perceive, plan, code, execute, and validate complex manipulation tasks through a closed-loop control cycle, which is developed by https://github.com/Jasonxu1225 .
Key Features
Three specialized agents working in coordination:
Key Related Files
embodichain/agents/hierarchy/embodichain/lab/gym/envs/tasks/tableware/embodichain/toolkits/interfaces.pyembodichain/lab/gym/motion_generation/embodichain/agents/README.mdType of change
Screenshots
Pour Water
pour_waterr.mp4
Rearrangement
rearrangement.mp4
Dual Arm Pour Water
pour_water_dual.mp4
Checklist
I have run the
black .command to format the code base.I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works
Dependencies have been updated, if applicable.