Skip to content

Feat/training report metadata - #992

Open
blugassi wants to merge 3 commits into
NVIDIA:mainfrom
blugassi:feat/training-report-metadata
Open

Feat/training report metadata#992
blugassi wants to merge 3 commits into
NVIDIA:mainfrom
blugassi:feat/training-report-metadata

Conversation

@blugassi

Copy link
Copy Markdown
Contributor

Summary

  • Expand training_report.json with schema versioning, run identity, environment, hardware, precision, parallelism, and configuration-source metadata.
  • Replace workload-specific report strategies with one scenario-level TrainingReporter.
  • Add validation for required TensorBoard and configuration artifacts.
  • Document schema version 1.0.

Test Plan

Tested on Linux with Python 3.12.

python -m pytest tests/report_generator/training/test_training_parser.py \
  tests/test_parser.py tests/test_init.py tests/test_test_scenario.py

Branch-focused result: 140 tests passed.

pre-commit run ruff-check --all-files
pre-commit run ruff-format --all-files

Both Ruff checks passed. Changed-file Pyright completed with zero errors.

Ran cloudai generate-report against existing NeMoRun, MegatronRun, and Megatron-Bridge results. All three generated valid reports containing the new metadata and resolved absolute configuration paths.

Additional Notes

Unsupported workloads and runs missing required artifacts are skipped with warnings. Configuration paths use empty strings when provenance is unavailable.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 758f6311-f1e9-41b5-80f4-0d6200fc6d88

📥 Commits

Reviewing files that changed from the base of the PR and between 1468060 and 243c357.

📒 Files selected for processing (1)
  • src/cloudai/report_generator/training/parser.py

📝 Walkthrough

Walkthrough

The PR defines a unified training report schema, adds configuration provenance to test scenarios, enriches training report parsing, and replaces strategy-based reporting with multi-run TrainingReporter registration.

Changes

Training report pipeline

Layer / File(s) Summary
Report schema and configuration contract
doc/training-report-schema.md, src/cloudai/report_generator/training/models.py, src/cloudai/report_generator/training/mappings.py
Defines schema version 1.0 and adds training configuration mappings for precision, expert parallelism, container images, and aggregation fields.
Scenario configuration provenance
src/cloudai/_core/test_scenario.py, src/cloudai/core.py, src/cloudai/parser.py, tests/test_parser.py
Adds public ConfigPaths and attaches resolved configuration paths to parsed test scenarios.
Scenario-aware training parsing
src/cloudai/report_generator/training/parser.py, tests/report_generator/training/test_training_parser.py
Builds training configuration from scenario metadata, environment variables, host and node data, GPU counts, model mappings, and clique size.
Multi-run reporter registration
src/cloudai/report_generator/training/reporter.py, src/cloudai/report_generator/training/__init__.py, src/cloudai/registration.py, tests/test_init.py, tests/test_test_scenario.py
Replaces the previous training report strategy with TrainingReporter, which processes supported test runs independently and writes reports.

Estimated code review effort: 4 (Complex) | ~45 minutes

Suggested reviewers: jj10306

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly summarizes the training report metadata, reporter changes, validation, documentation, and testing included in the changeset.
Title check ✅ Passed The title concisely identifies the main change: adding training report metadata and schema-related enhancements.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/cloudai/report_generator/training/parser.py (1)

97-102: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Handle unreadable configuration artifacts.

get_model_config can raise OSError or UnicodeDecodeError after the file-existence check. These exceptions escape can_parse. TrainingReporter.generate calls can_parse before its try block, so one unreadable report artifact stops processing of later runs instead of logging a skip warning.

Proposed fix
-            except (json.JSONDecodeError, yaml.YAMLError) as exc:
+            except (OSError, UnicodeDecodeError, json.JSONDecodeError, yaml.YAMLError) as exc:
                 logging.warning(f"{name}: invalid config artifact at '{config_path}' ({exc}); skipping training report")
                 return False
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cloudai/report_generator/training/parser.py` around lines 97 - 102,
Update can_parse around the get_model_config call to also catch OSError and
UnicodeDecodeError, logging the same skip warning used for malformed JSON/YAML
artifacts. Ensure unreadable configuration files return the non-parseable result
so TrainingReporter.generate can continue processing subsequent runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cloudai/report_generator/training/reporter.py`:
- Around line 42-44: In the parser selection flow, add a warning log immediately
before the continue when parser_cls is None, identifying the skipped unsupported
workload using tr.test.test_template_name. Keep the existing skip behavior
unchanged.

---

Outside diff comments:
In `@src/cloudai/report_generator/training/parser.py`:
- Around line 97-102: Update can_parse around the get_model_config call to also
catch OSError and UnicodeDecodeError, logging the same skip warning used for
malformed JSON/YAML artifacts. Ensure unreadable configuration files return the
non-parseable result so TrainingReporter.generate can continue processing
subsequent runs.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 772e9e84-6dde-4407-b145-b39d24eae80f

📥 Commits

Reviewing files that changed from the base of the PR and between 5b5c0f7 and 1468060.

📒 Files selected for processing (14)
  • doc/training-report-schema.md
  • src/cloudai/_core/test_scenario.py
  • src/cloudai/core.py
  • src/cloudai/parser.py
  • src/cloudai/registration.py
  • src/cloudai/report_generator/training/__init__.py
  • src/cloudai/report_generator/training/mappings.py
  • src/cloudai/report_generator/training/models.py
  • src/cloudai/report_generator/training/parser.py
  • src/cloudai/report_generator/training/reporter.py
  • tests/report_generator/training/test_training_parser.py
  • tests/test_init.py
  • tests/test_parser.py
  • tests/test_test_scenario.py

Comment thread src/cloudai/report_generator/training/reporter.py
Include identity, environment, hardware, precision, and parallelism details so training reports capture the resolved run context.

Signed-off-by: Ben Lugassi <blugassi@nvidia.com>
Generate per-run training reports from a scenario reporter so parsers can include scenario-level context.

Signed-off-by: Ben Lugassi <blugassi@nvidia.com>
Signed-off-by: Ben Lugassi <blugassi@nvidia.com>
@blugassi
blugassi force-pushed the feat/training-report-metadata branch from 1468060 to 243c357 Compare August 10, 2026 12:13

### 0.0

Initial schema.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to have a visualized block of the current class definition here at the very end using autoclass and members directives pointing to MetricStats (see other rst files for reference)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yes, please convert the whole file into rst format

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants