Skip to content

Feat/privileged obs refactor - #47

Merged
TATP-233 merged 12 commits into
mainfrom
feat/privileged-obs-refactor
Mar 18, 2026
Merged

TATP-233 merged 12 commits into
mainfrom
feat/privileged-obs-refactor

Conversation

@TATP-233

Copy link
Copy Markdown
Collaborator

No description provided.

TATP-233 and others added 12 commits March 18, 2026 04:11
…ults

Remove all default values from RewardConfig dataclasses to force explicit
Hydra configuration. This prevents silent parameter mismatches during training.

Changes:
- Remove default_factory from RewardConfig/RewardConfigPPO dataclasses
- Add runtime validation in training scripts (train_rsl_rl, train_offpolicy, train_appo, train_mlx_ppo)
- Cache reward_config in env __init__ as self._reward_cfg for performance
- Add pose_weights to g1_ppo_mujoco.yaml
- Update env configs to use reward_config: Type | None = None

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ward config

- Fix registry.py to properly handle Union types (e.g., RewardConfig | None)
- Add _reward_cfg cache to G1WalkTaskMjSAC
- Add default reward config fixtures in conftest.py
- Update tests to provide reward_config or expect ValueError
- Replace obsolete default-matching tests with skip markers
- All tests passing (403 passed, 4 skipped)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The config file has target_entropy_ratio: 0.0, not 0.2.
Update test to match actual config value.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ssion

- Add split_obs_dict() and get_obs_dims() utilities
- Update IPC layer: SharedOnPolicyStorage and ReplayBuffer support privileged_dim
- Rename env obs key: 'actor' → 'obs' for semantic clarity
- Fix reset() signature: 3-tuple → 2-tuple (remove duplicate obs)
- Update workers: split obs dict, write obs and privileged separately
- Update learners: actor uses obs only, critic uses obs+privileged
- Update runners: detect and pass privileged_dim to IPC and workers
- Update train_rsl_rl.py: adapt to new obs key naming

Breaking changes:
- Environments must use 'obs' key instead of 'actor'
- reset() now returns (obs_dict, info) instead of (obs, obs, info)
…tion logic

1. Remove defaults from RewardConfigSAC (consistency with other envs)
2. Add runtime validation in G1WalkTaskMjSAC.__init__
3. Update G1 SAC YAML configs with all required fields
4. Extract reward injection to utils/reward_utils.py
5. Update all training scripts to use extract_reward_config()
6. Fix get_env_dims to accept env_cfg_override
7. Update tests with G1 SAC fixture

All tests passing (404 passed, 4 skipped)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add tests for split_obs_dict() and get_obs_dims()
- Fix test envs: rename 'actor' → 'obs', fix reset() signature
- Update all test assertions to use new obs key naming
Fixes compatibility issues after merging privileged obs refactor:

- Fix Allegro envs reset() to return 2-tuple (obs_dict, info)
- Fix APPO worker: import flatten_obs_dict, handle privileged in final_observation
- Fix APPO runner: exclude last_privileged from transpose, init critic with obs+privileged dims
- Fix APPO learner: cache critic_obs_td, use separate obs for actor/critic
- Fix RSL-RL wrapper: use "actor" key, handle "obs"/"privileged" keys
- Fix test wrappers: adapt to 2-tuple reset(), provide reward_config fixtures
- Fix train scripts: use get_obs_dims(), extract obs only for actor, handle 2-tuple reset()

All tests pass (250 default, 68 slow, 6 veryslow). Training validated for PPO, MLX-PPO, APPO.
- Add g1_appo_mujoco.yaml and go2_appo_mujoco.yaml reward configs
- Add reward overrides to all APPO task configs (go1/go2/g1_joystick)
- Add g1_joystick.yaml task alias for offpolicy (maps to g1_sac)
- Add veryslow tests to validate all task configs load correctly

Tests now catch missing reward configs that would cause runtime failures.
All 3 APPO locomotion tasks (go1/go2/g1_joystick) now start successfully.
- Add privileged_dim parameter to FastSACLearner __init__
- Actor uses obs_dim only, critic uses obs_dim + privileged_dim
- Fix FastSACRunner to use get_obs_dims() instead of sum()
- Fix offpolicy worker to use get_obs_dims() for actor initialization
- Fix update_actor to pass obs+privileged to critic
- Fix train_offpolicy.py to extract reward_config before creating env
- Remove incorrect g1_joystick task config for offpolicy

SAC/TD3 now work correctly with privileged observations architecture.
- Create go2_sac_mujoco.yaml reward config
- Update go2_joystick task to use go2_sac_mujoco reward

All offpolicy task configs now load successfully.
@TATP-233
TATP-233 merged commit 972a15c into main Mar 18, 2026
0 of 10 checks passed
@TATP-233
TATP-233 deleted the feat/privileged-obs-refactor branch March 18, 2026 05:48
TATP-233 added a commit that referenced this pull request May 4, 2026
TATP-233 added a commit that referenced this pull request Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant