Repository navigation
Add G1 visual locomotion training and deployment pipeline - #8776
Double7sBurger wants to merge 17 commits into
Conversation
github-actions
Bot
added
documentation
 [High impact] The PR adds a new path from simulation training to physical G1 motor control. Do not merge the hardware deployment path until its timing, stop behavior, stale-state checks, operator controls, and camera trust are fixed. Findings
SummaryAdds G1 teacher training, depth-student distillation, LEAPP export support, and a physical-robot deployment script.
Diagram%%{init: {'theme': 'neutral'}}%%
flowchart LR
T[Teacher PPO training] --> C[Teacher checkpoint]
C --> D[Depth-student distillation]
D --> S[Student checkpoint]
S --> E[LEAPP export]
E --> G[Deployment graph]
R[Robot joint and IMU readings] --> G
K[Keyboard commands] --> G
U[UDP depth frames] --> G
G --> M[Physical motor targets]
Reviews (1) · Last reviewed commit: "Clarify G1 training and deployment guide..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| self._next += period | ||
| time.sleep(max(0.0, self._next - time.monotonic())) |
There was a problem hiding this comment.
G1Link.wait() counts from construction time, before the three-second take_lowcmd() probe, camera startup, and operator confirmation. Its deadline is already several seconds behind when the ramp starts. The ramp then sends successive targets without the intended waits while the deadline catches up, moving the robot faster than the advertised three seconds. Reset the deadline when control starts and skip missed deadlines rather than replaying them.
| bring_up(link, policy.default_pose, policy.kp, policy.kd, ramp_s=3.0) | ||
|
|
||
| try: |
There was a problem hiding this comment.
bring_up() runs outside the try/finally that calls link.damp(). Pressing an abort combination during the ramp, hold, or engagement countdown raises OperatorAbort and exits without sending damping commands. An upright-check failure or Ctrl+C during these stages also skips cleanup. Move bring_up() inside the protected block so stopping during startup sends the promised damping response.
| def _on_state(self, message) -> None: | ||
| self.state = message | ||
|
|
||
| def read(self) -> dict: | ||
| """Joint state and IMU, in the frames the graph expects.""" | ||
| state = self.state |
There was a problem hiding this comment.
G1Link keeps the last rt/lowstate message forever without checking its age. If DDS state updates stop while depth frames continue, the policy keeps sending motor targets based on old joint angles and IMU readings. The fall and remote-abort checks also read that old message. Record the receive time and stop with damping when robot state becomes stale, including during bring-up.
| print("\n>>> HOLDING the start pose. Square the robot up and lower it onto its feet.") | ||
| print(">>> KEEP IT SUPPORTED: this is PD with no policy, and the pose is not statically stable.") | ||
| held = threading.Event() | ||
| threading.Thread(target=lambda: (input(">>> type 'go2' and press Enter: "), held.set()), daemon=True).start() |
There was a problem hiding this comment.
The second confirmation accepts any line, not just go2: held.set() runs unconditionally after input(). Pressing Enter or typing an unrelated response starts the engagement countdown while the operator may still be positioning the robot. Check the entered word before setting the event and send rejected confirmations through the damping cleanup.
| the measured orientation. Steering a heading rather than a rate keeps a teleoperated run on | ||
| the distribution a fixed command trains on. | ||
| """ | ||
| key = sys.stdin.read(1) if select.select([sys.stdin], [], [], 0.0)[0] else None |
There was a problem hiding this comment.
Teleop.command() reads single characters, but the script never switches the terminal out of its normal line-buffered mode. With the documented launch command, w, space, and x do not reach this code until Enter is pressed. The advertised space-to-stop control therefore does not stop on a keypress. Enable single-key terminal input after the startup prompts and restore the terminal on exit.
| payload = self._sock.recv(1 << 16) | ||
| except TimeoutError: | ||
| continue | ||
| if len(payload) < count * 2: | ||
| continue | ||
| raw = np.frombuffer(payload, dtype=np.uint16, count=count, offset=len(payload) - count * 2) |
There was a problem hiding this comment.
DepthStream binds every network interface and accepts frames from any sender. A host that can reach the UDP port can repeatedly send fabricated distances. Each packet replaces the camera view and refreshes _at, preventing the stale-camera stop even when the real publisher is down. The policy uses these frames to produce physical motor targets. Restrict the listener to the intended network path and authenticate the publisher before accepting frames or refreshing their age.
How this was verified: Any sufficiently large UDP packet replaces the depth frame and its freshness time, and the control loop passes that frame into the policy before writing its targets to the motors.
| ssh unitree@192.168.123.164 \ | ||
| '/usr/bin/python3 ~/depth_publisher.py --host <WORKSTATION_IP>' |
There was a problem hiding this comment.
The camera instructions run ~/depth_publisher.py, but the PR neither supplies that file nor explains where to obtain it. Deployment waits for its frames, and the guide requires a particular crop and image size. A new user cannot complete this setup from the provided instructions. Include the publisher or link to an available implementation with its setup and crop settings.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
|
||
| **Quaternion order.** The IMU publishes ``(w, x, y, z)`` and the graph expects ``(x, y, z, w)``. | ||
|
|
||
| **IMU mounting offset.** Subtracted before the quaternion reaches the graph. |
There was a problem hiding this comment.
The guide says the IMU mounting offset is subtracted before inference, but G1Link.read() only reorders the quaternion and passes it through. Users may skip a needed correction because they are told it already happens. Remove this claim or document and implement the calibration step.
![isaaclab-review-bot[bot]](https://avatars.githubusercontent.com/u/157846462?s=60&v=4){kind=small}
There was a problem hiding this comment.
Isaac Lab Review Bot
Needs attention: Design and architecture (2), Scope and over-engineering (1), Implementation (5), Style consistency (2). Other reviewed categories look good.
- PR description: Correct the checkpoint claim: enforcement covers every
DistillationRunner; add baseline/candidate raycast-reward throughput with revisions, command, hardware, backend, environment count, warmup, and step-time metric. - Design and architecture: Config construction downloads and parses USD, while the reward initializes private sensor state; move derivation and initialization to the existing asset and sensor lifecycle owners.
- Scope and over-engineering: Minimal implementation: extend
rough_env_cfg.py, shared MDP owners, and RSL-RL exporters; keepsymmetry/g1.pyandg1_deploy.pyfor distinct contracts, and keepdistillation.pywhile collapsingAugmentedStorageintoMirrorDistillation. Suggested PR breakdown: 1) shared MDP terms/tests; 2) RSL-RL guard/export fixes; 3) G1 configs, symmetry, distillation/tests; 4) hardware script and guide. - API: API changes look good.
- Compatibility and deprecation: No breaking changes identified.
- Implementation: Hardware bring-up sits outside damping cleanup and accepts any engage response, so aborts can leave active commands or start walking unexpectedly; guard bring-up and require
go2. - Style consistency: The deployment entry point uses forbidden prefixed prints for status; configure repository console logging and reserve
printfor operator prompts. - Test quality: Test quality looks good.
Needs rework. Posted 10 inline findings.
Automated review; human maintainers own approval decisions.
| print("\n>>> HOLDING the start pose. Square the robot up and lower it onto its feet.") | ||
| print(">>> KEEP IT SUPPORTED: this is PD with no policy, and the pose is not statically stable.") | ||
| held = threading.Event() | ||
| threading.Thread(target=lambda: (input(">>> type 'go2' and press Enter: "), held.set()), daemon=True).start() |
There was a problem hiding this comment.
🔴 Critical · Implementation — Any input engages the policy
The hold thread's lambda calls input(...) then held.set() unconditionally, so a blank or mistyped line engages the walking policy. Validate that the response equals go2, as confirm does.
| print(f"[ok] mode_machine={link.read()['mode_machine']} policy at {1 / CONTROL_DT:.0f} Hz") | ||
| print(f"[..] teleop: {Teleop.HELP}") | ||
| teleop = Teleop() | ||
| bring_up(link, policy.default_pose, policy.kp, policy.kd, ramp_s=3.0) |
There was a problem hiding this comment.
🔴 Critical · Implementation — Bring-up runs outside damping cleanup
bring_up commands motors before the try/finally, so an abort, interrupt, or the upright check exits without damp, contradicting its "every exit goes through here" contract. Move it inside the guarded block.
| "joint_pos": np.array([m.q for m in motors[: len(MOTOR_NAMES)]], dtype=np.float32), | ||
| "joint_vel": np.array([m.dq for m in motors[: len(MOTOR_NAMES)]], dtype=np.float32), | ||
| # The IMU publishes (w, x, y, z); the graph declares (x, y, z, w). | ||
| "root_quat_xyzw": quat[[1, 2, 3, 0]], |
There was a problem hiding this comment.
🟡 Warning · Implementation — IMU mounting offset never applied
read only reorders quaternion components, while the guide states the mounting offset is subtracted before the quaternion reaches the graph. Apply the calibrated transform here or correct the documentation.
|
|
||
| def damp(self, kd, seconds: float = 1.5) -> None: | ||
| """Bleed off energy with no stiffness, then stop commanding. Every exit goes through here.""" | ||
| print("[..] damping down") |
There was a problem hiding this comment.
🟡 Warning · Style Consistency — Use logging instead of prefixed prints
Status and failure output uses print with [ok]/[..]/[cmd] prefixes, which the contribution guide forbids. Configure console logging and emit logger.info/logger.error, keeping print only for operator prompts.
| return actions[..., self.perm] * self.sign | ||
|
|
||
|
|
||
| class AugmentedStorage: |
There was a problem hiding this comment.
🟡 Warning · Scope Complexity — Delete single-consumer storage wrapper
AugmentedStorage only forwards attributes and overrides generator(), whose sole consumer is the overridden update(). Apply the mirroring inline there and delete the wrapper and its handoff.
| yield out | ||
|
|
||
|
|
||
| class MirrorDistillation(Distillation): |
There was a problem hiding this comment.
🟡 Warning · Style Consistency — Document and annotate the new algorithm
MirrorDistillation is selected through a public resolvable string yet has no docstrings or type hints; symmetry/g1.py likewise omits module and public-function documentation that anymal.py provides. Add Google-style docstrings and annotations.
| for name in ("pyramid_stairs", "pyramid_stairs_inv"): | ||
| terrain.sub_terrains[name].step_height_range = (0.05, 0.35) | ||
| terrain.sub_terrains[name].step_width = 0.3 | ||
| self.actions.joint_pos.scale, sole_size, sole_offset = get_action_parameters(self.scene.robot) |
There was a problem hiding this comment.
🟡 Warning · Design Architecture — Config post-init downloads and parses USD
get_action_parameters runs inside __post_init__, so merely constructing this config retrieves and composes a USD stage. Keep configs declarative: declare the scales and sole geometry, or resolve them at environment construction.
| sensor = env.scene[sensor_cfg.name] | ||
| positions = robot.data.body_pos_w.torch[:, asset_cfg.body_ids] | ||
| quaternions = robot.data.body_quat_w.torch[:, asset_cfg.body_ids] | ||
| xy = torch.cartesian_prod( |
There was a problem hiding this comment.
🟡 Warning · Implementation — Reward rebuilds static geometry each step
linspace, cartesian_prod, and the sole size/offset tensors are reallocated on every step for 4096 environments. Precompute the device-local sole points once and reuse them in this hot path.
| if key not in scanner.meshes: | ||
| from isaaclab.sensors.ray_caster.base_ray_caster import BaseRayCaster | ||
|
|
||
| BaseRayCaster._initialize_warp_meshes(scanner) |
There was a problem hiding this comment.
🟡 Warning · Design Architecture — Reward initializes sensor's private meshes
This per-step reward hardcodes scene["height_scanner"] and calls private BaseRayCaster._initialize_warp_meshes, taking over the sensor's lifecycle. Let the sensor own initialization and pass the ray caster through a scene-entity config.
| ) | ||
| if not torch.equal(perm[perm], torch.arange(len(names), device=device)): | ||
| raise RuntimeError("the joint permutation is not an involution") | ||
| env._g1_joint_mirror_map = (perm, sign) |
There was a problem hiding this comment.
🟡 Warning · Implementation — Cache mirrors without mutating the env
_build_map injects _g1_joint_mirror_map and siblings onto the environment through getattr/attribute assignment. Sibling anymal.py caches the same permutation and sign tensors with functools.cache keyed by joint names and device; reuse that.
ooctipus
added
the
ci:run-docker
This branch has not been deployed
2 participants
Description
Add a G1 visual locomotion workflow covering teacher training, depth-student distillation, LEAPP export, and Unitree DDS deployment. Includes reusable core MDP terms, G1 symmetry, curriculum scheduling, asset-derived action parameters, and a training/deployment guide. The canonical rough G1 task uses the teacher configuration; scene, rewards, events, curriculum, terminations, and student observations are composed declaratively.
Require a teacher checkpoint for custom distillation algorithms and export only the selected inference observation groups. Deployment dependencies and camera setup are documented in the guide.
Rough PPO consumes
proprioandheight_scan; the depth student consumesproprio_historyanddepth. The shared G1 terrain preset lives in its own configuration module.Flat G1 now exposes
proprioand supports RSL-RL only. Update group overrides frompolicytoproprioand select--rl_library rsl_rl; the flat SKRL entry point and recipe have been removed.Type of change
Validation
Existing TorchScript and actuator deprecation warnings remain. Hardware deployment performance has not been validated. The Newton collision fix is tracked separately in newton-physics/newton#4870 and is not included here.
Checklist
uv run isaaclab --format