Add machine-readable scenario summary report#919
Conversation
Generate cloudai-summary.json as a default scenario reporter so automation can discover scenario status, report artifacts, test run artifacts, and configured metrics without scraping workload-specific files. Issue: NVIDIA#917 Tested: uv run ruff check src/cloudai/reporter.py src/cloudai/registration.py src/cloudai/_core/registry.py src/cloudai/core.py tests/test_reporter.py tests/test_init.py Tested: uv run ruff format --check src/cloudai/reporter.py src/cloudai/registration.py src/cloudai/_core/registry.py src/cloudai/core.py tests/test_reporter.py tests/test_init.py Tested: uv run pytest Signed-off-by: shreyaskommuri <shreyaskommuri@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a new SummaryReporter that writes a machine-readable cloudai-summary.json; registers and exports the reporter; adjusts report ordering to place "summary" before "tarball"; and adds tests that assert registry ordering and the produced JSON structure. ChangesSummaryReporter: Machine-Readable Summary Artifact
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
| summary_path = slurm_system.output_path / SummaryReporter.SUMMARY_FILE_NAME | ||
| summary = json.loads(summary_path.read_text()) | ||
|
|
||
| assert summary["schema_version"] == "1.0" |
There was a problem hiding this comment.
I'd rather do a full object assert like
assert summary == {
"schema_version": "1.0",
...
}| def build_summary(self) -> dict[str, Any]: | ||
| test_runs = [self._test_run_summary(tr) for tr in self.trs] | ||
| return { | ||
| "schema_version": "1.0", |
There was a problem hiding this comment.
I think this property is a bit too much atm, let's keep the set of fields to minimum. Only those that bring value
|
|
||
| return [ | ||
| self._artifact(path) | ||
| for path in sorted(self.results_root.iterdir()) |
There was a problem hiding this comment.
this won't work gracefully when we run sweeping. in the sweeping case, the folders structure will be
/scenario foler/
/case 1/
/sweep 1/
/some-report.html
/sweep 2/
/some-report.html
There was a problem hiding this comment.
sweeping is actually quite important in our work. I believe this json summary report should bring insights if those were run
Trim the default scenario summary to the fields external tools need and group DSE/sweep outputs under their parent test run so nested sweep artifacts remain discoverable. Constraint: Reviewer requested a smaller JSON surface, a full-object regression assertion, and sweep-aware artifact reporting. Rejected: Keeping iteration, step, description, schema_version, and system metadata in the first summary contract | they widen the surface before consumers have asked for them. Confidence: high Scope-risk: narrow Directive: Treat cloudai-summary.json as an automation entry point; add fields only when they have clear consumer value. Tested: uv run ruff check src/cloudai/reporter.py tests/test_reporter.py tests/test_init.py src/cloudai/registration.py src/cloudai/_core/registry.py src/cloudai/core.py Tested: uv run ruff format --check src/cloudai/reporter.py tests/test_reporter.py tests/test_init.py src/cloudai/registration.py src/cloudai/_core/registry.py src/cloudai/core.py Tested: uv run pytest tests/test_reporter.py tests/test_init.py tests/test_registry.py -q Tested: uv run pytest -q Not-tested: Live CloudAI sweep run on an HPC system.
|
Addressed the requested changes and updated the PR branch from latest main. What changed:
Validation on the updated PR head dfa28da:
|
Summary
summaryscenario reporter that writescloudai-summary.jsonnext to the existing scenario reports.tarballas the final scenario reporter.Fixes #917.
Test Plan
Environment:
uv runCommands and results:
Additional Notes