-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix(robomimic): unify config loading logic to avoid FileNotFoundError in training #3981
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?
fix(robomimic): unify config loading logic to avoid FileNotFoundError in training #3981
Conversation
…to avoid FileNotFoundError in training
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 fixes a FileNotFoundError in robomimic imitation learning training by unifying configuration loading logic with the existing load_cfg_from_registry utility.
Key Changes:
- Extended
load_cfg_from_registryinparse_cfg.pyto support.jsonconfiguration files (previously only.yamlwas supported) - Added null check for
mod.__file__to prevent errors with namespace packages - Updated
train.pyto use the unified loader instead of custom file opening logic - Added type checking to ensure robomimic configs are dictionaries
- Removed duplicate module path resolution code
Impact:
- Robomimic training now properly supports
"module:file"config path format (e.g.,isaaclab_tasks.manager_based.manipulation.stack.config.franka.agents:robomimic/bc_rnn_low_dim.json) - Configuration loading is now consistent between RL (RSL-RL) and IL (robomimic) training pipelines
- The fix eliminates the error that occurred when trying to open the module path string directly
Confidence Score: 5/5
- This PR is safe to merge with minimal risk
- The changes are well-structured and solve a real bug. The unified
load_cfg_from_registryapproach is already proven in RSL-RL training. The extension to support JSON is straightforward, and the null check formod.__file__is a defensive improvement. Type checking ensures robomimic receives expected dictionary format. No breaking changes to existing functionality. - No files require special attention
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| source/isaaclab_tasks/isaaclab_tasks/utils/parse_cfg.py | 5/5 | Extended load_cfg_from_registry to support JSON files alongside YAML, with proper module path resolution and null check for mod.__file__ |
| scripts/imitation_learning/robomimic/train.py | 5/5 | Replaced direct file opening with unified load_cfg_from_registry utility, added type checking for dict format, removed duplicate module resolution logic |
Sequence Diagram
sequenceDiagram
participant User
participant train.py
participant gym_registry as Gym Registry
participant load_cfg as load_cfg_from_registry
participant importlib
participant filesystem as File System
participant robomimic as Robomimic Config
User->>train.py: Run training with task name
train.py->>train.py: Build cfg_entry_point_key<br/>(e.g., "robomimic_bc_cfg_entry_point")
train.py->>load_cfg: load_cfg_from_registry(task_name, cfg_entry_point_key)
load_cfg->>gym_registry: Get cfg_entry_point from task kwargs
gym_registry-->>load_cfg: Return "module:file.json" path
alt File has module:path format
load_cfg->>importlib: import_module(mod_name)
importlib-->>load_cfg: Return module object
load_cfg->>load_cfg: Get module.__file__ and dirname
load_cfg->>filesystem: Resolve full path to JSON file
else Absolute path
load_cfg->>filesystem: Use path directly
end
load_cfg->>filesystem: open(config_file)
filesystem-->>load_cfg: File contents
alt File is .json
load_cfg->>load_cfg: json.load(f)
else File is .yaml
load_cfg->>load_cfg: yaml.full_load(f)
end
load_cfg-->>train.py: Return config dict
train.py->>train.py: Type check: isinstance(ext_cfg, dict)
train.py->>robomimic: config_factory(ext_cfg["algo_name"])
robomimic-->>train.py: Base config object
train.py->>train.py: config.update(ext_cfg)
train.py->>User: Begin training with loaded config
1 file reviewed, no comments
Signed-off-by: Litian Gong <[email protected]>
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
Fixes FileNotFoundError in robomimic training when config paths use "module:file" format.
Changes:
- Extended
load_cfg_from_registry()inparse_cfg.pyto support JSON files (previously only YAML) - Refactored
train.pyto use the unified config loader instead of manual path resolution - Added type validation to ensure robomimic configs are dictionaries
- Now consistent with RL training pipeline (rsl_rl, rl_games, etc.) which already use this utility
Impact:
- Eliminates code duplication between RL and IL config loading
- Maintains backward compatibility with existing YAML configs
- Properly resolves module-based paths like
isaaclab_tasks.manager_based.manipulation.stack.config.franka.agents:robomimic/bc_rnn_low_dim.json
Confidence Score: 5/5
- This PR is safe to merge with minimal risk
- The changes are well-contained bug fixes that unify config loading logic. The implementation properly handles edge cases (absolute paths, module paths, different file formats), maintains backward compatibility with existing YAML configs, and adds appropriate error handling. The refactoring eliminates duplicate code and aligns robomimic training with the existing RL training pattern.
- No files require special attention
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| source/isaaclab_tasks/isaaclab_tasks/utils/parse_cfg.py | 5/5 | Extended load_cfg_from_registry to support JSON files alongside YAML. Added conditional JSON loading with proper module path resolution and error handling. |
| scripts/imitation_learning/robomimic/train.py | 5/5 | Replaced manual config path resolution and JSON loading with unified load_cfg_from_registry utility. Added type check to ensure dict format for robomimic compatibility. |
Sequence Diagram
sequenceDiagram
participant User
participant train.py
participant load_cfg_from_registry
participant gym.spec
participant importlib
participant json_module
participant config_factory
User->>train.py: Run training with task name
train.py->>load_cfg_from_registry: Call with task_name and cfg_entry_point_key
load_cfg_from_registry->>gym.spec: Get kwargs from registry
gym.spec-->>load_cfg_from_registry: Return cfg_entry_point (e.g., "module:file.json")
alt cfg_entry_point ends with .json or .yaml
alt Absolute path exists
load_cfg_from_registry->>load_cfg_from_registry: Use path as-is
else Module path format
load_cfg_from_registry->>importlib: Import module by name
importlib-->>load_cfg_from_registry: Return module
load_cfg_from_registry->>load_cfg_from_registry: Resolve file path from module
end
alt .json file
load_cfg_from_registry->>json_module: json.load(file)
json_module-->>load_cfg_from_registry: Return dict
else .yaml file
load_cfg_from_registry->>load_cfg_from_registry: yaml.full_load(file)
end
else Python class
load_cfg_from_registry->>load_cfg_from_registry: Instantiate class
end
load_cfg_from_registry-->>train.py: Return ext_cfg (dict)
train.py->>train.py: Validate ext_cfg is dict
train.py->>config_factory: Create config from ext_cfg["algo_name"]
config_factory-->>train.py: Return robomimic config object
train.py->>train.py: Update config with ext_cfg values
train.py-->>User: Training starts with loaded config
3 files reviewed, no comments
|
In short:
|
Description
This PR fixes a FileNotFoundError raised during robomimic imitation learning training when
the configuration file path is provided in the
"module:file"format.Issue:
train.pydirectly opened the module path string usingopen(), which failed to resolvethe actual file path. RL (e.g. rsl_rl) training lib already uses
load_cfg_from_registryto correctlyparse this syntax, but IL (robomimic) training bypassed that utility.
Solution:
load_cfg_from_registryto support.jsonconfiguration files.robomimic/train.pyto use the unified loader instead ofopen().Fixes #3980
Type of change
Screenshots
N/A — change only affects config loading logic, no UI impact.
Checklist
pre-commitchecks with./isaaclab.sh --formatextension.tomlCONTRIBUTORS.mdor it already existsAcceptance Criteria
"module:file"style config paths.load_cfg_from_registryimplementation insource/isaaclab_tasks/isaaclab_tasks/utils/parse_cfg.py