-
Couldn't load subscription status.
- Fork 2.6k
Adds Hydra configuration preset overriding features. #3785
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Adds Hydra configuration preset overriding features. #3785
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR introduces Hydra group override functionality to IsaacLab, enabling runtime configuration switching through command-line arguments. The feature allows users to define reusable configuration variants (e.g., different observation schemes, action modes, or network architectures) in their config classes and swap them using simple CLI flags like env.observations=noise_less or agent.policy=large_network. The implementation adds two new core functions (register_hydra_group() and resolve_hydra_group_runtime_override()) to the Hydra utilities module, modifies the Franka reach environment to demonstrate variants for observations and actions, provides agent variants for network architectures, and includes comprehensive documentation and tests.
PR Description Notes:
- The changelog date (2025-10-15) appears incorrect given the current date is2025-10-21
Potential Issues
-
Critical - Lambda variable capture bug (
source/isaaclab_tasks/isaaclab_tasks/utils/hydra.py:169-175): The lambda functionsget_variantsandis_group_variantare defined inside a loop and will capture loop variables by reference, binding to their final values rather than current iteration values. This is a classic Python closure pitfall that could cause incorrect variant resolution.# Current problematic code: for sec, cfg in (("env", env_cfg), ("agent", agent_cfg)): get_variants = lambda c: getattr(c, "variants", None) or ... is_group_variant = lambda k, v: k.startswith(pref) and ...
Solution: Define these functions outside the loop or use default arguments to capture current values:
get_variants = lambda c, s=sec: getattr(c, "variants", None) or ...
-
Moderate - In-place config mutation (
source/isaaclab_tasks/isaaclab_tasks/utils/hydra.py:180-181): The code deletes thevariantsattribute from config objects after processing, which could break code that expects this field to persist. Consider documenting this behavior or making copies instead. -
Moderate - Inconsistent naming convention (
source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/reach/reach_env_cfg.py:120): The class is namedNoiselessObservationsCfgbut the variant key is"noise_less"(snake_case). While functional, this inconsistency could confuse users about naming conventions for variants. -
Low - Action space compatibility (
source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/reach/config/franka/joint_pos_env_cfg.py:46-57): The action variants define different action types and scales but don't verify action space dimensions remain compatible. Runtime errors could occur if variants produce mismatched action dimensions. -
Low - Documentation clarity (
docs/source/features/hydra.rst:77): The module path changed fromomni.isaac.labtoisaaclab, but this isn't explained or called out in the documentation. Verify this reflects the actual codebase structure.
Confidence Score
2 out of 5 - The lambda variable capture bug in the core utility function is a critical issue that could cause unpredictable behavior in production. The feature implementation is sound conceptually, but this fundamental Python error needs immediate attention before merge. Once fixed, thorough testing is recommended to ensure variants resolve correctly across different scenarios.
9 files reviewed, 3 comments
| for section in ("env", "agent"): | ||
| section_dict = cfg_dict.get(section, {}) | ||
| if isinstance(section_dict, dict) and "variants" in section_dict: | ||
| for root_name, root_dict in section_dict["variants"].items(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this conflict with variants inside the USD file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this doesn't, the variants key is only detected at top level, env.variants, agent.variants.
Then all inner group are using the dotted path to add variants, for example, env.variants.scene.actions.arm_action is understood to be a group preset that bind to env.scene.actions.arm_action
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if you pick a variant and then want to override its setting? Does this work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes it will work
so for example,
env.actions=joint_position_action <------------- group preset override
env.actions.arm_action=relative_joint_position <-------------- group preset override
env.action.arm_action.use_zero_offset=False <------------ regular override
as long as the order of the cli is correct it will pick up the right override
| self.commands.ee_pose.body_name = "panda_hand" | ||
| self.commands.ee_pose.ranges.pitch = (math.pi, math.pi) | ||
|
|
||
| variants = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have this only in the test case and not main codebase?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I can do that for sure,
though it will be nice to have a more visible place(maybe in future) to show how to use group override.
I do think it is pretty nice that you can do
env.actions.arm_action=joint_position_to_limit
env.actions.arm_action=relative_joint_position
instead of being super explicit in task ID
| Declare alternatives under ``self.variants`` in your environment and agent configs. Each top-level key under | ||
| ``variants`` becomes a Hydra group (``env.<key>`` or ``agent.<key>``), and each nested key is a selectable option. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this doesn't work if variants are inside the inner configs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it currently only binds variants to env and agent, and all inner config are specified through dotted path. I actually thought about maybe introduce a more generalized variant class so you can bind not only in top level env or agent, but can do it at any inner level. But It feels like will be quite invasive, at need to think about it a lot more carefully. If something nice come out, I think providing both dotted path, or inner cfg variant can be pretty nice.
Description
This PR add group override feature from Hydra to IsaacLab, this feature has the potential to save us a lot of Task ID
With this PR, in addition of overriding primitive functions and values, you can pre-defined override-able configuration.
For example:
Defining environment variants:
Overriding syntax
This variants is also the same for all rl-framework.
rslrl example:
RL-games example
rsl_rl overrides cli : agent.policy=large_network
rl_games overrides cli : agent.params.network.mlp=large_network
Type of change
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there