Fixes Rlinf config path#6137
Conversation
Greptile SummaryThis PR fixes the RLinf config YAML path resolution in both
Confidence Score: 5/5Minimal, targeted change — only the config-dir resolution expression is touched in two files; all surrounding logic is unchanged. Both changes correctly delegate to No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Script starts
play_rlinf.py / train_rlinf.py] --> B{--config_path
provided?}
B -- Yes --> C[Use explicit config_path]
B -- No --> D{isaaclab_tasks
package found?}
D -- Yes --> E[rglob config_name.yaml
within package tree]
E --> F{Match
found?}
F -- Yes --> G[Return parent dir
of first match]
F -- No --> H[Fall back to
script directory]
D -- No --> H
C --> I[Build RLINF_CONFIG_FILE
env var path]
G --> I
H --> I
I --> J[Hydra initialize_config_dir
+ compose]
Reviews (1): Last reviewed commit: "fix config path" | Re-trigger Greptile |
There was a problem hiding this comment.
Review Summary
Verdict: Clean, focused bug fix. Looks good overall.
This PR correctly replaces the hardcoded fallback directories (SCRIPT_DIR in play_rlinf.py and RLINF_DIR in train_rlinf.py) with calls to cli_args.resolve_config_dir(), which searches the isaaclab_tasks package tree for the config YAML before falling back to the script directory. This ensures that config files located in the task packages are properly discovered — fixing path resolution for task configurations that have been moved/reorganized.
✅ What's Good
- Consistent pattern: Both files now use the same
resolve_config_dir()resolution logic, eliminating divergent fallback behavior. - Non-breaking: The
resolve_config_dirfunction gracefully falls back to the script directory (same as before) when the config isn't found inisaaclab_tasks, so existing workflows using--config_pathor local configs remain unaffected. - Correct ordering: The
config_nameassignment is moved aboveconfig_dirsinceresolve_config_dirneeds it as input — good catch.
📝 Minor Observations
-
No changelog fragment: The checklist shows the changelog/version update is unchecked. For a bug fix that changes runtime path resolution, a fragment under "Fixed" (e.g., "Fixed RLinf config YAML resolution to search
isaaclab_taskspackage tree") would help downstream users understand why their configs now resolve correctly. -
Fallback behavior difference: In the old code,
play_rlinf.pyfell back toSCRIPT_DIR(therlinf/directory) whiletrain_rlinf.pyfell back toRLINF_DIR(also therlinf/directory, computed differently). Theresolve_config_dirfunction uses_SCRIPT_DIR(samerlinf/directory) as its final fallback. The behavior is preserved, but it's worth noting the unification is intentional.
Verdict
The fix is minimal, correct, and well-scoped. No functional concerns.
Description
Fixes path of RLinf config yaml file caused by missing path resolver.
Type of change
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there