Repository navigation
Feat/reward config injection - #46
Merged
Merged
Conversation
- Add structured_configs.py with typed dataclasses (SACConfig, TD3Config, APPOConfig, PPOConfig) - Create conf/ directory with 18 YAML files for offpolicy/appo/ppo configs - Rewrite all 4 training scripts to use @hydra.main() instead of argparse - Strip locomotion_params.py and manipulation_params.py to KNOWN_TASKS only - Remove ConfigDict compat code from appo/runner.py - Update all tests to use structured configs and Hydra composition - Replace ml_collections with hydra-core>=1.3 in pyproject.toml - Fix play_interactive.py and benchmark_fast_sac_backends.py Benefits: - Type-safe configs with IDE autocomplete - YAML-based config management with version control - CLI override support (e.g., algo.num_envs=2048) - Flexible task/algo composition via Hydra defaults All 106 tests pass.
- Replace argparse CLI examples with Hydra syntax - Add config system section to CLAUDE.md - Add config guidelines to CONTRIBUTING.md - Update all training command examples (task=go1_joystick, algo.num_envs=2048, etc.)
Implement Hydra-based reward configuration system supporting backend-specific (MuJoCo/Motrix) and algo-specific (SAC/TD3/APPO/RSL-RL) reward parameters. Core changes: - Registry: Add type-driven dict→dataclass conversion (15 lines) - Training scripts: Extract reward config from Hydra, pass to runners - Runners/workers: Thread env_cfg_override through multiprocess pipeline - Configs: Add 12 YAML files for task/algo/backend combinations - Tests: Add 11 tests covering config loading and integration All algorithms verified with actual training runs.
- Add @pytest.mark.veryslow for full training iteration tests, excluded by default even from `make test-slow` - Expand env tests from 4 instantiation-only to 7 envs with full reset + step verification (dict obs keys, shapes, reward, done) - Add RSL-RL PPO 1-iteration test for Go2 and G1 environments - Add MLX PPO 1-iteration test on real Go2 env (macOS only) - Mark SAC/TD3 learn tests as veryslow - Fix APPO learn test: use real env instead of DummyFlatTest which is not registered in the collector subprocess (mp.spawn), causing 60s timeout per iteration instead of actual training - Add early crash detection in APPORunner.learn() — fail fast when collector subprocess dies instead of waiting through 60s timeouts - Fix RSL-RL wrapper _obs_to_tensordict to include "privileged" key in TensorDict so rsl_rl v5 resolve_obs_groups can find it
…tion Resolve conflicts between: - feat/reward-config-injection: Hydra config system, reward config injection - main (PR #45): obs dict refactor (NpEnvState.obs: ndarray → dict[str, ndarray]) Conflict resolutions: - Use cfg.training.* refs (Hydra) from our branch; obs_groups_spec + flatten_obs_dict from main - Keep KNOWN_TASKS-only locomotion/manipulation_params (our Hydra migration) - Keep Hydra tests; drop tests for deleted config factory functions - fast_sac/runner: pass env_cfg_override (ours) + obs_groups_spec (main) - play_interactive: keep PPOConfig import (ours) + flatten_obs_dict (main) - train_rsl_rl: keep play_rsl_rl(cfg, device) Hydra signature (ours)
- train_rsl_rl.py play_rsl_rl(): rebuild env_cfg_override locally from cfg.reward (was referencing undefined variable from outer scope) - conf/offpolicy/task/*.yaml: move # @Package _global_ before defaults: so Hydra applies task overrides to global config (not nested under cfg.task.*) - conf/offpolicy/config.yaml: put reward before task in defaults list so task files can use 'override /reward:' syntax - conf/offpolicy/task/*.yaml: use 'override /reward:' syntax to avoid duplicate reward config error - conf/appo/config.yaml: move _self_ to first in defaults list so task overrides (max_iterations, save_interval) correctly take precedence
- ruff format: reformat train_rsl_rl.py and rotation.py - rotation.py: fix reset() return type annotation to match NpEnv base class signature (Tuple[np.ndarray, ...] → Tuple[dict[str, np.ndarray], ...]) after structured-obs-dict migration changed obs type - benchmark_fast_sac_backends.py: remove unused collector_device variable (F841 ruff lint error)
- veryslow marker + comprehensive training integration tests (APPO, RSL-RL, MLX) - env reset+step tests with dict obs key/shape/reward/done verification - APPORunner early crash detection for dead collector subprocess - RslRlVecEnvWrapper: add "privileged" key to TensorDict for rsl_rl v5 compat Conflict resolution: - test_appo_runner: use APPOConfig() instead of deleted appo_config() factory - test_rsl_rl_runner: use PPOConfig() instead of deleted ppo_config() factory
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
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.
No description provided.