Skip to content

Conversation

@Gonglitian
Copy link
Contributor

@Gonglitian Gonglitian commented Nov 9, 2025

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.py directly opened the module path string using open(), which failed to resolve
the actual file path. RL (e.g. rsl_rl) training lib already uses load_cfg_from_registry to correctly
parse this syntax, but IL (robomimic) training bypassed that utility.

Solution:

  • Extended load_cfg_from_registry to support .json configuration files.
  • Updated robomimic/train.py to use the unified loader instead of open().
  • Now both RL and IL training pipelines share a consistent configuration loading mechanism.

Fixes #3980


Type of change

  • Bug fix (non-breaking change which fixes an issue)

Screenshots

N/A — change only affects config loading logic, no UI impact.


Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation (if required)
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the changelog and version in the corresponding extension.toml
  • I have added my name to CONTRIBUTORS.md or it already exists

Acceptance Criteria

  • Robomimic training now supports "module:file" style config paths.
  • Config loading logic is unified with updated load_cfg_from_registry implementation in source/isaaclab_tasks/isaaclab_tasks/utils/parse_cfg.py

@Gonglitian Gonglitian requested a review from Mayankm96 as a code owner November 9, 2025 02:11
@github-actions github-actions bot added bug Something isn't working isaac-mimic Related to Isaac Mimic team labels Nov 9, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a 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_registry in parse_cfg.py to support .json configuration files (previously only .yaml was supported)
  • Added null check for mod.__file__ to prevent errors with namespace packages
  • Updated train.py to 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_registry approach is already proven in RSL-RL training. The extension to support JSON is straightforward, and the null check for mod.__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
Loading

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a 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() in parse_cfg.py to support JSON files (previously only YAML)
  • Refactored train.py to 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
Loading

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@Gonglitian
Copy link
Contributor Author

In short:

  1. I expanded load_cfg_from_registry so it can resolve robomimic style .json cfg file now.
  2. so in robomimic/train.py we can just use function load_cfg_from_registry to keep train.py clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working isaac-mimic Related to Isaac Mimic team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug Report] FileNotFoundError when loading robomimic config via module path

1 participant