refactor: extract HumanoidRobot base class; deduplicate H1/H1_2/G1 env files#86
Open
dcol91863 wants to merge 3 commits intounitreerobotics:mainfrom
Open
refactor: extract HumanoidRobot base class; deduplicate H1/H1_2/G1 env files#86dcol91863 wants to merge 3 commits intounitreerobotics:mainfrom
dcol91863 wants to merge 3 commits intounitreerobotics:mainfrom
Conversation
…or heightfield/trimesh Closes unitreerobotics#81 The `_update_terrain_curriculum(env_ids)` method was referenced in the docstring of `reset_idx` but was never implemented, making terrain curriculum training a no-op even when `cfg.terrain.curriculum = True`. Changes ------- legged_gym/envs/base/legged_robot.py: 1. _get_env_origins() — heightfield / trimesh branch (new) Previously the method only created a flat grid regardless of terrain type, so `custom_origins` was always False and robots were never placed on the actual heightfield platforms. Now: - Sets `custom_origins = True` for heightfield and trimesh terrains. - Converts `terrain.env_origins` (numpy, rows=levels, cols=types) to a GPU tensor `self.terrain_origins` for fast index lookups. - Assigns each environment a random terrain type (column). - Initialises each environment to a random level in [0, max_init_terrain_level]. - Sets `self.env_origins` from the lookup table. 2. _update_terrain_curriculum(env_ids) — new method Promotes robots that traversed more than half a platform length (env_length / 2) during the episode to the next harder level, and demotes robots that fell short by one level. Levels are clamped to [0, num_rows - 1]. env_origins is refreshed for the reset envs. The method is a no-op on the very first call (before init_done) so the initial level assignment from _get_env_origins is preserved. 3. reset_idx() — wired up Added call to `_update_terrain_curriculum(env_ids)` when `cfg.terrain.curriculum` is True (placed before robot repositioning so the updated origin is used immediately). Also logs the mean terrain level to `extras["episode"]["terrain_level"]` for monitoring.
…terrain, helpers, BaseTask ABC
Fixes found during a broad audit of the codebase. Each change is
independent; grouped here to avoid PR noise.
---
legged_gym/envs/base/legged_robot.py
_push_robots() velocity corruption (critical)
- The previous code wrote random XY velocities into self.root_states[:]
(all num_envs rows) but only committed push_env_ids rows back to the
simulator. This left stale random values in the root_states tensor for
non-pushed environments, corrupting base_lin_vel / base_ang_vel
observations on the same step.
- Fix: index the write with push_env_ids, matching the indexed set call.
Duplicate / unused imports removed
- LEGGED_GYM_ROOT_DIR was imported twice (lines 1 and 14).
- envs (line 1), WarningMessage (line 3), and Tensor (line 11) were
imported but never referenced anywhere in the file.
---
deploy/deploy_mujoco/deploy_mujoco.py + configs/{g1,h1,h1_2}.yaml
Missing max_cmd factor — sim2real observation mismatch (critical)
- deploy_real builds cmd obs as: cmd * cmd_scale * max_cmd
- deploy_mujoco built it as: cmd * cmd_scale (max_cmd missing)
- The policy was trained with the former, so the MuJoCo deployment was
feeding a different numerical range for the command slice, leading to
degraded or incorrect behaviour.
- Fix: read max_cmd from config and apply it; add max_cmd field to all
three MuJoCo YAML configs (values match deploy_real configs).
---
legged_gym/utils/terrain.py
selected_terrain() three bugs fixed
- length= was set to width_per_env_pixels instead of length_per_env_pixels,
producing square sub-terrains even when env_length != env_width.
- vertical_scale= and horizontal_scale= referenced self.vertical_scale /
self.horizontal_scale, attributes that do not exist on Terrain; the
correct references are self.cfg.vertical_scale / self.cfg.horizontal_scale
(consistent with how make_terrain() accesses the same values).
- eval(terrain_type)(terrain, **self.cfg.terrain_kwargs.terrain_kwargs)
had a double .terrain_kwargs; changed to **self.cfg.terrain_kwargs.
curiculum → curriculum spelling fix
- Method name curiculum() and its call site corrected to curriculum().
---
legged_gym/utils/helpers.py
get_load_path() bare except and month-boundary sort bug
- Bare except: caught all exceptions including KeyboardInterrupt and
MemoryError, swallowing the original error and replacing it with a
misleading message. Now uses except OSError and chains the cause.
- Empty-directory case was an IndexError (runs[-1]) silently converted
to the same misleading message; now checked explicitly.
- Lexicographic sort breaks at month/year boundaries (e.g. Oct > Nov
alphabetically). Now sorts by mtime so the most-recent run is always
picked correctly regardless of name format.
---
legged_gym/envs/base/base_task.py
BaseTask should be abstract
- BaseTask had reset_idx() and step() that raised NotImplementedError at
runtime. A subclass that forgot to implement either would only fail when
the method was actually called.
- Added ABC base class and @AbstractMethod decorators so Python raises
TypeError at instantiation time instead.
… H1/H1_2/G1 env files The three humanoid robot environments (H1, H1_2, G1) were near-complete copies of each other — 10 methods, ~120 lines, were pasted verbatim in every file. The only code that legitimately differed was the hip-joint DOF indices used in _reward_hip_pos(). Changes ------- legged_gym/envs/base/humanoid_robot.py (new file) Introduces HumanoidRobot(LeggedRobot) — a shared base class that owns: - _init_foot() / _init_buffers() (rigid-body foot state setup) - update_feet_state() (per-step foot state refresh) - _post_physics_step_callback() (gait phase computation) - _get_noise_scale_vec() (observation noise scaling) - compute_observations() (obs + privileged_obs assembly) - _reward_contact() (phase-gated contact reward) - _reward_feet_swing_height() (swing foot height penalty) - _reward_alive() (constant survival reward) - _reward_contact_no_vel() (contact-while-moving penalty) - _reward_hip_pos() (uses self.hip_indices — set per subclass) legged_gym/envs/h1/h1_env.py legged_gym/envs/h1_2/h1_2_env.py legged_gym/envs/g1/g1_env.py Each reduced to a 3-line subclass that sets hip_indices. Zero behaviour change — the logic is identical, just deduplicated. legged_gym/scripts/play.py Removed duplicate imports of sys and LEGGED_GYM_ROOT_DIR (lines 1-5 contained each import twice). Removed unused RECORD_FRAMES and MOVE_CAMERA variables (set in __main__ but never read anywhere). legged_gym/envs/base/legged_robot_config.py + legged_gym/utils/terrain.py Corrected long-standing spelling mistake: slope_treshold → slope_threshold (both the config field definition and the call site in terrain.py).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The three humanoid robot environments (
H1Robot,H1_2Robot,G1Robot) were near-complete copies of each other. 10 methods, ~120 lines were pasted verbatim in all three files. The only code that legitimately differed was the hip-joint DOF indices used in_reward_hip_pos().This PR extracts the shared logic into a new
HumanoidRobot(LeggedRobot)intermediate base class, reducing each robot env file to a 3-line subclass.New file:
legged_gym/envs/base/humanoid_robot.pyHumanoidRobotowns all methods common to the three robots:_init_foot()/_init_buffers()update_feet_state()_post_physics_step_callback()_get_noise_scale_vec()compute_observations()_reward_contact()_reward_feet_swing_height()_reward_alive()_reward_contact_no_vel()_reward_hip_pos()self.hip_indices— set per subclassReduced robot env files
Each of
h1_env.py,h1_2_env.py,g1_env.pyis now:Zero behaviour change — the logic is identical to what was previously duplicated, just deduplicated.
Additional fixes in this PR
play.py: Removed duplicate imports ofsysandLEGGED_GYM_ROOT_DIR(both appeared twice on lines 1–5). Removed unused variablesRECORD_FRAMESandMOVE_CAMERA(set in__main__block but never read).legged_robot_config.py+terrain.py: Corrected long-standing spelling mistakeslope_treshold→slope_thresholdin both the config field and the call site that reads it.Test plan
python legged_gym/scripts/train.py --task h1— confirm training starts and rewards match previous behaviourpython legged_gym/scripts/train.py --task h1_2— samepython legged_gym/scripts/train.py --task g1— samepython legged_gym/scripts/play.py --task h1— confirm play works and policy exports correctlyslope_thresholdis read correctly whenmesh_type="trimesh"is set