Adds support for custom `ProgressReporter` to Ray integration
Description
This PR adds support for providing a custom ProgressReporter while doing hyperparameter tuning with Ray Integration.
Without the PR, the Ray integration defaults to the standard CLIReporter, which often displays metrics that aren’t particularly relevant or at the desired frequency. Similar to how we allow users to specify a --cfg_class (e.g., CartpoleTheiaJobCfg), this PR lets them optionally provide a custom ProgressReporter class. If such is not provided, it falls back to the default.
Moreover, I have added an example inside vision_cartpole_cfg.py (i.e., CustomCartpoleProgressReporter).
One point to highlight is that the new "context-aware progress reporting" conflicts with custom ProgressReporter, so if a custom ProgressReporter is provided, the PR disables the context-aware progress reporting.
Fixes #3268.
Type of change
- New feature (non-breaking change which adds functionality)
Checklist
- [x] I have run the
pre-commitchecks with./isaaclab.sh --format - [ ] I have made corresponding changes to the documentation
- [x] 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 the corresponding version in the extension's
config/extension.tomlfile - [x] I have added my name to the
CONTRIBUTORS.mdor my name already exists there
@garylvov, would love to get your feedback if you have time!
Hi thanks for the PR! (EDIT: didn't mean to mention the issue got this confused with early stopping)
Greptile Overview
Greptile Summary
This PR adds support for custom ProgressReporter classes in Ray hyperparameter tuning, allowing users to customize metrics display and reporting frequency. The implementation:
- Adds optional
--progress_reporterCLI argument that accepts a class name from the config file - Disables Ray's context-aware progress reporting when custom reporter is used (via
RAY_AIR_NEW_OUTPUT=0) - Validates that custom reporters don't set
_metricor_modeattributes directly - Provides example implementation (
CustomCartpoleProgressReporter) showing typical usage
Issues found:
- Line 544: If user specifies a non-existent progress reporter class, code silently falls back to default instead of raising an error
- Line 546-549: Doesn't handle case where user passes an already-instantiated reporter object (will incorrectly raise TypeError)
Confidence Score: 3/5
- This PR has logical errors in error handling that will cause confusing behavior for users
- Score reflects two critical logic bugs: (1) silently ignoring invalid reporter class names instead of raising errors, leading to confusing silent fallback behavior, and (2) incorrectly rejecting already-instantiated reporter objects. These will cause user-facing issues but don't affect core functionality when valid class names are provided.
- Pay close attention to scripts/reinforcement_learning/ray/tuner.py lines 544-549 for the error handling logic
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| scripts/reinforcement_learning/ray/tuner.py | 3/5 | Adds custom ProgressReporter support with validation logic, but has error handling gaps when class not found and doesn't handle instance inputs |
| scripts/reinforcement_learning/ray/hyperparameter_tuning/vision_cartpole_cfg.py | 5/5 | Adds example CustomCartpoleProgressReporter class demonstrating the feature - implementation looks correct |
Sequence Diagram
sequenceDiagram
participant User
participant CLI as tuner.py CLI
participant Module as Config Module
participant Tuner as invoke_tuning_run
participant Ray as Ray Tune
User->>CLI: Run with --progress_reporter CustomReporter
CLI->>Module: Load config file
CLI->>Module: getattr(module, "CustomReporter")
alt Class not found
Module-->>CLI: hasattr returns False
CLI->>CLI: progress_reporter = None (silent fallback)
else Class found
Module-->>CLI: Return CustomReporter class
CLI->>CLI: Validate is subclass of ProgressReporter
CLI->>CLI: Instantiate reporter
end
CLI->>Tuner: invoke_tuning_run(cfg, args, progress_reporter)
alt Custom reporter provided
Tuner->>Tuner: Set RAY_AIR_NEW_OUTPUT=0
Tuner->>Tuner: Validate no _metric/_mode attributes
end
Tuner->>Ray: Create RunConfig with progress_reporter
Ray->>Ray: Start tuning with custom reporter
@kellyguo11 pinging on small diff PR that only touches Ray stuff that's been open for some while, can we please merge? Thank you!!!