Repository navigation
Conversation
8c338f5 to
2fde3a6
Compare
There was a problem hiding this comment.
Pull request overview
This PR aligns G1 joystick (PPO/SAC) with the Go2-style kp/kd domain randomization flow by introducing per-joint actuator gain randomization via a formal backend capability (get_actuator_gains()), and wiring that through the locomotion DR provider/reset payload generation.
Changes:
- Add an optional
SimBackend.get_actuator_gains()contract and implement it forMuJoCoBackendto read per-actuator kp/kd from the model. - Introduce
G1DomainRandConfig+ G1 DR provider gain caching, and pass cached base kp/kd into common reset randomization utilities. - Update Motrix task configs to explicitly disable kp/kd randomization (avoiding unsupported-backend initialization errors).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/unilab/envs/locomotion/g1/joystick.py | Adds G1-specific DR config/provider hook; conditionally caches per-joint gains from backend during env init. |
| src/unilab/envs/locomotion/g1/joystick_sac.py | Switches SAC config to G1 DR config and mirrors the gain-caching DR initialization path. |
| src/unilab/envs/locomotion/common/dr_provider.py | Adds _get_base_actuator_gains() hook and threads base kp/kd into validate/reset-plan generation. |
| src/unilab/dr/dr_utils.py | Extends common reset randomization builder/validator with optional base_kp/base_kd and removes implicit backend probing. |
| src/unilab/base/backend/base.py | Defines optional backend API get_actuator_gains() with a default NotImplementedError. |
| src/unilab/base/backend/mujoco_backend.py | Implements get_actuator_gains() by reading MuJoCo actuator gain/bias parameters. |
| conf/ppo/task/g1_joystick/motrix.yaml | Disables kp/kd DR under Motrix for PPO. |
| conf/offpolicy/task/sac/g1_sac/motrix.yaml | Disables kp/kd DR under Motrix for SAC. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if cfg.domain_rand.randomize_kp or cfg.domain_rand.randomize_kd: | ||
| base_kp, base_kd = backend.get_actuator_gains() | ||
| dr_provider = G1JoystickDomainRandomizationProvider(base_kp=base_kp, base_kd=base_kd) | ||
| else: | ||
| dr_provider = G1JoystickDomainRandomizationProvider() | ||
| self._init_domain_randomization(dr_provider) | ||
|
|
There was a problem hiding this comment.
The domain-randomization provider selection logic is duplicated here and in g1/joystick_sac.py. To reduce the chance of future drift (e.g., adding a new DR term in one env but not the other), consider factoring this into a shared helper (or a small method on the base env) that returns the correctly-configured provider.
| if cfg.domain_rand.randomize_kp or cfg.domain_rand.randomize_kd: | |
| base_kp, base_kd = backend.get_actuator_gains() | |
| dr_provider = G1JoystickDomainRandomizationProvider(base_kp=base_kp, base_kd=base_kd) | |
| else: | |
| dr_provider = G1JoystickDomainRandomizationProvider() | |
| self._init_domain_randomization(dr_provider) | |
| dr_provider = self._create_domain_randomization_provider(backend) | |
| self._init_domain_randomization(dr_provider) | |
| def _create_domain_randomization_provider( | |
| self, backend: Any | |
| ) -> G1JoystickDomainRandomizationProvider: | |
| if self._cfg.domain_rand.randomize_kp or self._cfg.domain_rand.randomize_kd: | |
| base_kp, base_kd = backend.get_actuator_gains() | |
| return G1JoystickDomainRandomizationProvider(base_kp=base_kp, base_kd=base_kd) | |
| return G1JoystickDomainRandomizationProvider() |
| self._init_reward_functions() | ||
| self._init_domain_randomization(G1JoystickDomainRandomizationProvider()) | ||
| if cfg.domain_rand.randomize_kp or cfg.domain_rand.randomize_kd: | ||
| base_kp, base_kd = backend.get_actuator_gains() |
There was a problem hiding this comment.
For clarity and to avoid relying on a local variable after initialization, consider calling self._backend.get_actuator_gains() instead of backend.get_actuator_gains() here. It makes it explicit that the gains are being read from the env’s active backend instance.
| base_kp, base_kd = backend.get_actuator_gains() | |
| base_kp, base_kd = self._backend.get_actuator_gains() |
| if cfg.domain_rand.randomize_kp or cfg.domain_rand.randomize_kd: | ||
| base_kp, base_kd = backend.get_actuator_gains() | ||
| dr_provider = G1JoystickDomainRandomizationProvider(base_kp=base_kp, base_kd=base_kd) | ||
| else: | ||
| dr_provider = G1JoystickDomainRandomizationProvider() | ||
| self._init_domain_randomization(dr_provider) | ||
|
|
There was a problem hiding this comment.
The domain-randomization provider selection logic is duplicated with g1/joystick.py. Consider extracting a shared helper so PPO/SAC variants stay in sync when DR behavior changes (e.g., additional reset terms or different gain-caching rules).
| if cfg.domain_rand.randomize_kp or cfg.domain_rand.randomize_kd: | |
| base_kp, base_kd = backend.get_actuator_gains() | |
| dr_provider = G1JoystickDomainRandomizationProvider(base_kp=base_kp, base_kd=base_kd) | |
| else: | |
| dr_provider = G1JoystickDomainRandomizationProvider() | |
| self._init_domain_randomization(dr_provider) | |
| dr_provider = self._create_domain_randomization_provider(cfg, backend) | |
| self._init_domain_randomization(dr_provider) | |
| def _create_domain_randomization_provider(self, cfg: G1JoystickSACCfg, backend): | |
| """Create the DR provider while staying aligned with PPO/shared logic when available.""" | |
| parent_factory = getattr(super(), "_create_domain_randomization_provider", None) | |
| if callable(parent_factory): | |
| return parent_factory(cfg, backend) | |
| if cfg.domain_rand.randomize_kp or cfg.domain_rand.randomize_kd: | |
| base_kp, base_kd = backend.get_actuator_gains() | |
| return G1JoystickDomainRandomizationProvider(base_kp=base_kp, base_kd=base_kd) | |
| return G1JoystickDomainRandomizationProvider() |
| self._init_reward_functions() | ||
| self._init_domain_randomization(G1JoystickDomainRandomizationProvider()) | ||
| if cfg.domain_rand.randomize_kp or cfg.domain_rand.randomize_kd: | ||
| base_kp, base_kd = backend.get_actuator_gains() |
There was a problem hiding this comment.
For readability, prefer self._backend.get_actuator_gains() over the local backend variable here, so it’s clear the gains are read from the env’s configured backend instance.
| base_kp, base_kd = backend.get_actuator_gains() | |
| base_kp, base_kd = self._backend.get_actuator_gains() |
|
再补充一下修改前后的训练对比曲线 |
There was a problem hiding this comment.
mujoco_uni.batch_env
pool.get_field(env_id, name) -> np.ndarray
提供了批量获取DR参数属性的功能,替换成这个接口
feat: G1 per-joint kp/kd domain randomization (issue #129)
feat: G1 per-joint kp/kd domain randomization (issue #129)
feat: G1 per-joint kp/kd domain randomization (issue #129)
Closes Motphys#1617. UniSim 1.7.x accepts a bounded, readback-validated PhysX solver override set on the IsaacSim backend (unilabsim/unisim#251, Motphys#259), but the standard UniLab path (registry.make -> make_manager_based_rl_env -> create_backend) had no channel for those knobs: task owners could not declare the solver table and the backend silently ran PhysX defaults. - EnvCfg gains the six solver fields, following the per-backend knob convention (Optional, None keeps the PhysX scene defaults): isaacsim_solver_position_iteration_count (int >= 1), isaacsim_solver_velocity_iteration_count (int >= 0), isaacsim_bounce_threshold_velocity / isaacsim_contact_offset / isaacsim_rest_offset / isaacsim_max_depenetration_velocity (non-negative floats). EnvCfg.validate mirrors the PhysxSolverConfig bounds so owner configs fail fast. - env_backend_kwargs forwards all six to the UniSim factory under the same names. - Tests: field->kwargs mapping assertions (values forwarded verbatim, defaults stay None) and validation rejection coverage. - Changelog entry (shared page, Added).


Summary
Linked Work
Validation
make checkuv run pytest -m "not slow"Commands actually run:
uv run pyright src/unilab/base/backend/base.py \ src/unilab/base/backend/mujoco_backend.py \ src/unilab/dr/dr_utils.py \ src/unilab/envs/locomotion/common/dr_provider.py \ src/unilab/envs/locomotion/g1/joystick.py \ src/unilab/envs/locomotion/g1/joystick_sac.py uv run pytest -m "not slow"Impact
Checklist