Add G1 running policy trained with hydro-elastic contact model#25
Add G1 running policy trained with hydro-elastic contact model#25jnskkmhr wants to merge 9 commits into
Conversation
* add sand demo asset * add sand demo asset --------- Co-authored-by: Kamohara_Junnosuke_jkamohara3 <jkamohara3@MEL06551D.me.gatech.edu>
📝 WalkthroughWalkthroughAdds a new 29-DOF G1 robot variant with box-shaped feet: a full MJCF model, a matching RL policy YAML, and a README table entry documenting the new configuration. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @unitree_g1/mjcf/g1_29dof_rev_1_0_box_foot.xml:
- Around line 153-154: The file contains duplicate visual <geom> entries for the
same mesh/class on left_hip_roll_link: remove the redundant line so only a
single <geom> with mesh="left_hip_roll_link" and class="visual" remains (ensure
you keep the one with the correct attribute order/values if they differ); update
any nearby comments if necessary to reflect the deduplicated visual geometry for
left_hip_roll_link.
- Around line 137-139: Remove the duplicate visual geometry for the pelvis
contour: there are two <geom class="visual" mesh="pelvis_contour_link"/> entries
(one on the same line reversed attributes) causing the mesh to render twice;
keep a single <geom class="visual" mesh="pelvis_contour_link"/> entry and delete
the redundant one so only one visual geom references "pelvis_contour_link"
(ensure the remaining <geom> uses class="visual" and mesh="pelvis_contour_link"
consistently and retains ordering relative to the existing pelvis visual).
- Around line 147-148: There are two duplicate <geom> visual elements for the
left_hip_pitch_link mesh (one with material="black" and another inheriting the
visual class material), which likely causes unintended overlapping visuals;
either remove the redundant <geom> entry or, if a layered visual effect is
intended, add an explanatory comment and make the materials/attributes explicit
(e.g., unique names or opacity) so the purpose is clear — target the two <geom>
elements referencing mesh="left_hip_pitch_link" to resolve the duplication.
- Around line 397-399: The model references a geom named "floor" in the contact
pairs left_foot_floor and right_foot_floor but never defines that geom, so add a
floor geom definition matching other G1 models (name="floor", size="0 0 0.05",
type="plane", material="groundplane") into the worldbody (or global geom list)
of this MJCF file so the contact pairs can resolve; ensure the geom name is
exactly "floor" to match the left_foot_floor and right_foot_floor references.
🧹 Nitpick comments (1)
unitree_g1/mjcf/g1_29dof_rev_1_0_box_foot.xml (1)
410-416: Trailing whitespace in tendon coefficient values.Lines 412 and 428 have trailing whitespace in the
coefattribute (e.g.,coef="-0.2618 "). While MuJoCo may parse this correctly, it's inconsistent with other coefficient values. Consider removing the trailing space for consistency.Proposed fix
<fixed limited="true" range="-1e2 0.22846" solreflimit="0.003 1" solimplimit="0.99 0.999 0.0005 0 2" > <joint joint="left_ankle_roll_joint" coef="-0.14967"/> - <joint joint="left_ankle_pitch_joint" coef="-0.2618 "/> + <joint joint="left_ankle_pitch_joint" coef="-0.2618"/> </fixed>And similarly on line 428:
- <joint joint="right_ankle_pitch_joint" coef="-0.2618 "/> + <joint joint="right_ankle_pitch_joint" coef="-0.2618"/>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
unitree_g1/README.mdunitree_g1/mjcf/g1_29dof_rev_1_0_box_foot.xmlunitree_g1/rl_policies/g1_29DOF_rigid.ptunitree_g1/rl_policies/g1_29DOF_sand.ptunitree_g1/rl_policies/g1_29dof_rev_1_0.yaml
🔇 Additional comments (8)
unitree_g1/README.md (1)
13-13: LGTM!The new table entry for
g1_29dof_rev_1_0_box_footis correctly formatted and the DOF breakdown (6×2 legs + 3 waist + 7×2 arms = 29 DOF) is consistent with the MJCF model and YAML configuration.unitree_g1/mjcf/g1_29dof_rev_1_0_box_foot.xml (2)
436-470: LGTM!The actuator section correctly defines 29 motors matching all joints (6×2 legs + 3 waist + 7×2 arms). Control ranges are consistent with the joint force limits defined in the defaults.
472-479: LGTM!Sensor definitions are properly configured with references to valid site names defined in the worldbody.
unitree_g1/rl_policies/g1_29dof_rev_1_0.yaml (5)
17-55: LGTM!All 29 joint names are correctly listed and match the joint definitions in the MJCF file. The ordering (legs → waist → arms) is logical and consistent.
58-109: LGTM!All configuration arrays (
mjw_joint_pos,mjw_joint_stiffness,mjw_joint_damping,mjw_joint_armature) have exactly 29 elements matching the DOF count, with consistent grouping by body segment.
57-59: Verify initial pose consistency with MJCF.The
mjw_init_posz-value (0.8) differs slightly from the MJCF pelvis position (0.793). Additionally,mjw_init_quat: [0.0, 0.0, 0.7071, 0.7071]represents a 90° yaw rotation from the default orientation.If intentional (e.g., for specific demo scenarios), this is fine. Otherwise, consider aligning with the MJCF default pose.
98-109: Armature values differ from MJCF joint defaults.The YAML uses a uniform
0.01for all joint armatures, while the MJCF file specifies varying armature values per joint class (e.g., hip_roll: 0.025, ankle: 0.0072, shoulder: 0.0036).If the YAML armature is used to override MJCF values in simulation, this discrepancy could affect the robot dynamics and policy performance. Verify this is intentional for the policy's simulated environment.
111-118: LGTM!Observation parameters are well-defined with appropriate scale factors for an RL locomotion policy.
| <geom class="visual" material="black" mesh="pelvis"/> | ||
| <geom class="visual" mesh="pelvis_contour_link"/> | ||
| <geom mesh="pelvis_contour_link" class="visual"/> |
There was a problem hiding this comment.
Duplicate visual geometry for pelvis_contour_link.
Lines 138 and 139 both add the same pelvis_contour_link mesh as a visual geom. This duplication is likely unintentional and will cause the mesh to be rendered twice.
Proposed fix
<geom class="visual" material="black" mesh="pelvis"/>
<geom class="visual" mesh="pelvis_contour_link"/>
- <geom mesh="pelvis_contour_link" class="visual"/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <geom class="visual" material="black" mesh="pelvis"/> | |
| <geom class="visual" mesh="pelvis_contour_link"/> | |
| <geom mesh="pelvis_contour_link" class="visual"/> | |
| <geom class="visual" material="black" mesh="pelvis"/> | |
| <geom class="visual" mesh="pelvis_contour_link"/> |
🤖 Prompt for AI Agents
In @unitree_g1/mjcf/g1_29dof_rev_1_0_box_foot.xml around lines 137 - 139, Remove
the duplicate visual geometry for the pelvis contour: there are two <geom
class="visual" mesh="pelvis_contour_link"/> entries (one on the same line
reversed attributes) causing the mesh to render twice; keep a single <geom
class="visual" mesh="pelvis_contour_link"/> entry and delete the redundant one
so only one visual geom references "pelvis_contour_link" (ensure the remaining
<geom> uses class="visual" and mesh="pelvis_contour_link" consistently and
retains ordering relative to the existing pelvis visual).
| <geom class="visual" material="black" mesh="left_hip_pitch_link"/> | ||
| <geom material="black" mesh="left_hip_pitch_link" class="visual"/> |
There was a problem hiding this comment.
Duplicate visual geometry for left_hip_pitch_link.
Lines 147 and 148 both reference the same mesh. The first applies material="black", while the second inherits silver from the visual class. If this is intentional for a layered visual effect, consider adding a comment; otherwise, remove the duplicate.
Proposed fix if duplicate is unintentional
<geom class="visual" material="black" mesh="left_hip_pitch_link"/>
- <geom material="black" mesh="left_hip_pitch_link" class="visual"/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <geom class="visual" material="black" mesh="left_hip_pitch_link"/> | |
| <geom material="black" mesh="left_hip_pitch_link" class="visual"/> | |
| <geom class="visual" material="black" mesh="left_hip_pitch_link"/> |
🤖 Prompt for AI Agents
In @unitree_g1/mjcf/g1_29dof_rev_1_0_box_foot.xml around lines 147 - 148, There
are two duplicate <geom> visual elements for the left_hip_pitch_link mesh (one
with material="black" and another inheriting the visual class material), which
likely causes unintended overlapping visuals; either remove the redundant <geom>
entry or, if a layered visual effect is intended, add an explanatory comment and
make the materials/attributes explicit (e.g., unique names or opacity) so the
purpose is clear — target the two <geom> elements referencing
mesh="left_hip_pitch_link" to resolve the duplication.
| <geom class="visual" mesh="left_hip_roll_link"/> | ||
| <geom mesh="left_hip_roll_link" class="visual"/> |
There was a problem hiding this comment.
Duplicate visual geometry for left_hip_roll_link.
Lines 153 and 154 both add the same mesh as visual geometry.
Proposed fix
<geom class="visual" mesh="left_hip_roll_link"/>
- <geom mesh="left_hip_roll_link" class="visual"/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <geom class="visual" mesh="left_hip_roll_link"/> | |
| <geom mesh="left_hip_roll_link" class="visual"/> | |
| <geom class="visual" mesh="left_hip_roll_link"/> |
🤖 Prompt for AI Agents
In @unitree_g1/mjcf/g1_29dof_rev_1_0_box_foot.xml around lines 153 - 154, The
file contains duplicate visual <geom> entries for the same mesh/class on
left_hip_roll_link: remove the redundant line so only a single <geom> with
mesh="left_hip_roll_link" and class="visual" remains (ensure you keep the one
with the correct attribute order/values if they differ); update any nearby
comments if necessary to reflect the deduplicated visual geometry for
left_hip_roll_link.
| <pair name="left_foot_floor" geom1="left_foot_collision" geom2="floor" solref="0.01 1" friction="0.8 0.8"/> | ||
| <pair name="right_foot_floor" geom1="right_foot_collision" geom2="floor" solref="0.01 1" friction="0.8 0.8"/> | ||
| </contact> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for XML files containing "floor" definition
echo "=== Files with 'floor' in unitree_g1/mjcf/ ==="
rg -l "floor" unitree_g1/mjcf/ --type xml 2>/dev/null | head -20
echo -e "\n=== Search for g1_29dof_rev_1_0_box_foot usage ==="
rg "g1_29dof_rev_1_0_box_foot" . --type xml --type py 2>/dev/null | head -30
echo -e "\n=== Check if the file includes other files ==="
grep -n "include" unitree_g1/mjcf/g1_29dof_rev_1_0_box_foot.xml | head -20
echo -e "\n=== Check for floor geometry definition in target file ==="
rg "geom.*floor|name=\"floor\"" unitree_g1/mjcf/g1_29dof_rev_1_0_box_foot.xmlRepository: newton-physics/newton-assets
Length of output: 2638
🏁 Script executed:
#!/bin/bash
# Check the structure of the target file
echo "=== File structure (first 50 lines) ==="
head -50 unitree_g1/mjcf/g1_29dof_rev_1_0_box_foot.xml
echo -e "\n=== Check if any MJCF file defines a floor geom ==="
rg "name=\"floor\"" unitree_g1/mjcf/ --type xml
echo -e "\n=== Check for includes in the target file ==="
cat unitree_g1/mjcf/g1_29dof_rev_1_0_box_foot.xml | grep -i "include"
echo -e "\n=== Check if this file is included by other files ==="
rg "g1_29dof_rev_1_0_box_foot" --type xmlRepository: newton-physics/newton-assets
Length of output: 3968
🏁 Script executed:
#!/bin/bash
# Check file size and full structure to understand design intent
echo "=== Lines around contact section ==="
sed -n '390,410p' unitree_g1/mjcf/g1_29dof_rev_1_0_box_foot.xml
echo -e "\n=== Check if body section includes the robot structure ==="
grep -c "<body" unitree_g1/mjcf/g1_29dof_rev_1_0_box_foot.xml
echo -e "\n=== Check end of file to see complete structure ==="
tail -20 unitree_g1/mjcf/g1_29dof_rev_1_0_box_foot.xml
echo -e "\n=== Compare with a working model that defines floor ==="
echo "Lines around contact in g1_29dof_rev_1_0.xml:"
sed -n '390,410p' unitree_g1/mjcf/g1_29dof_rev_1_0.xml | head -15Repository: newton-physics/newton-assets
Length of output: 3122
Add missing floor geometry definition.
This file is a complete standalone MJCF model (unlike component files), yet it references a floor geom in contact pairs (lines 397-398) that is not defined anywhere in the file. All other G1 models in the same directory define floor with <geom name="floor" size="0 0 0.05" type="plane" material="groundplane"/>. Add the missing floor definition to ensure the model loads correctly in MuJoCo.
🤖 Prompt for AI Agents
In @unitree_g1/mjcf/g1_29dof_rev_1_0_box_foot.xml around lines 397 - 399, The
model references a geom named "floor" in the contact pairs left_foot_floor and
right_foot_floor but never defines that geom, so add a floor geom definition
matching other G1 models (name="floor", size="0 0 0.05", type="plane",
material="groundplane") into the worldbody (or global geom list) of this MJCF
file so the contact pairs can resolve; ensure the geom name is exactly "floor"
to match the left_foot_floor and right_foot_floor references.
eric-heiden
left a comment
eric-heiden
left a comment
There was a problem hiding this comment.
Thanks for your contribution @jnskkmhr! It looks good, can you please check the comments from CodeRabbit and then sign the CLA so we can get it merged?
2 participants
Description
This PR adds Unitree G1's locomotion policy trained with hydro-elastic contact model and updated MJCF asset file.
MJCF modification replaces 4 point collider with box collider.
Then, these files are used in
examples/mpm/example_mpm_g1_twoway_coupling.py.Added files are
unitree_g1/mjcf/g1_29dof_box_foot.xmlunitree_g1/rl_policies/g1_29DOF_sand.pt(trained with hydro-elastic contact model)unitree_g1/rl_policies/g1_29DOF_rigid.pt(trained with IsaacLab rigid contact model)unitree_g1/rl_policies/g1_29dof_rev_1_0.yamlBefore your PR is "Ready for review"
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.