Skip to content

Normalize cloned grad_absmax metrics for structured progress payloads - #2913

Merged
bghira merged 3 commits into
mainfrom
copilot/fix-code-review-comment
Jul 27, 2026
Merged

Normalize cloned grad_absmax metrics for structured progress payloads#2913
bghira merged 3 commits into
mainfrom
copilot/fix-code-review-comment

Conversation

Copilot AI commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Structured progress/webhook metrics were inconsistent between grad clipping modes: the "norm" path serialized cloned values to floats, while the "value" path could emit a Tensor for grad_absmax. This updates the value-clipping path to normalize cloned metrics the same way.

  • Metric serialization

    • Update Trainer._update_grad_metrics() so clone_norm_value=True converts grad_absmax to a detached float in the "value" clipping path.
    • Preserve existing behavior for accelerator logging paths that still use the original tensor object when cloning is not requested.
  • Focused coverage

    • Add a unit test for cloned grad_absmax serialization in the value-clipping path.
    • Update the progress-metrics expectation to assert the serialized float form for structured payload generation.
  • Behavioral example

    # Before: could place a Tensor into structured metrics
    trainer._update_grad_metrics(metrics, clone_norm_value=True)
    metrics["grad_absmax"]  # torch.tensor(...)
    
    # After: normalized for JSON-serializable payloads
    trainer._update_grad_metrics(metrics, clone_norm_value=True)
    metrics["grad_absmax"]  # 1.2

Copilot AI changed the title [WIP] Fix code based on review comment Normalize cloned grad_absmax metrics for structured progress payloads Jul 27, 2026
Copilot AI requested a review from bghira July 27, 2026 03:27
@bghira
bghira marked this pull request as ready for review July 27, 2026 03:37
@bghira
bghira merged commit db11a98 into main Jul 27, 2026
2 checks passed
@bghira
bghira deleted the copilot/fix-code-review-comment branch July 27, 2026 04:23
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