fix: map Harbor trial folders to eval case ids - #104
Conversation
--refine keyed trajectories by the raw trial directory name, but Harbor writes folders like case-001__Lmi47iy. Split off the suffix so trajectories.get(case id) actually finds them. Fixes NVIDIA#93 Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
rng1995
left a comment
rng1995
left a comment
There was a problem hiding this comment.
Reviewed the exact current head against #93 and the Harbor persisted-trial naming path. Suffixed folders map back to case ids and legacy unsuffixed folders remain unchanged. Focused tests pass (18), with Ruff and diff checks clean. No actionable findings; approved. The red Gitleaks job is unrelated branch history.
|
@mimran-khan - Please resolve merge conflicts |
|
Merged main in. Changelog conflict is resolved. |
# Conflicts: # CHANGELOG.md
|
Merged latest main (including #102) and resolved the CHANGELOG conflict. Ready for another look. |
|
Please resolve merge conflict in CHANGELOG.md so that I can merge it. Thanks for your contribution and patience @mimran-khan |
Head branch was pushed to by a user without write access
Merge main and prefer reward.json entry_id when mapping persisted trial folders to eval case ids. Use folder-name parsing only for unambiguous Harbor suffix tails; keep full folder names when metadata is missing.
|
Merged main and addressed the reward entry_id feedback at 5cb7de6. Ready for re-review. |
Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
Stop guessing Harbor suffix shape from folder names. Prefer reward entry_id, then result.json task metadata, and keep the full directory name when neither is present. Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
Resolve CHANGELOG.md after merging upstream NVIDIA#99, NVIDIA#107, and NVIDIA#112.
rng1995
left a comment
rng1995
left a comment
There was a problem hiding this comment.
Re-reviewed the exact 6043dc2 head. The canonical case ID now comes from persisted reward metadata, then Harbor result metadata, without unsafe suffix guessing; supported double-underscore IDs and ShortUUID trial folders are covered. The focused suite passes (24 tests), with Ruff and diff checks clean. Approved on code merits. GitHub currently reports a conflict with main, so that still needs to be resolved and the resulting head should complete CI before merge.
|
@mimran-khan - Please resolve merge conflicts. |
Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
Head branch was pushed to by a user without write access
3 participants
Summary
--refinekeyed trajectories by the raw Harbor trial folder name. Collection writescase-001__Lmi47iy, and eval cases arecase-001, so every lookup missed.Discovery now splits off the
__suffixand keys by the case id. A folder namedcase-001still works. Fixes #93.Verification
make lintmake testmake buildRelease Impact
CHANGELOG.md