NVIDIA / NVIDIA/Megatron-LM

Add regression test coverage for get_grad_norm_fp32 (norm_type paths + empty grads_for_norm)

Open
#5,827 0 comments 0 reactions 1 assignee Claimed by @gautham-kollu View on GitHub
community-request enhancement
Dominant language
Python
Stars
17.9k
Forks
4.5k
Avg merge
4d 3h
Merged PRs (30d)
272

Description

**Is your feature request related to a problem? Please describe.**

`tests/unit_tests/optimizer/test_clip_grads.py` contains exactly one test,
covering only the `OptimizerConfig.grad_norm_skip_threshold` default. There
is no test coverage for `get_grad_norm_fp32` itself — none of the three
norm_type branches (`2.0`, `inf`, generic p-norm) are exercised, and there
is no regression test for the empty `grads_for_norm` crash fixed in #5530.

**Describe the solution you'd like**

Add tests to `test_clip_grads.py` covering:
- `get_grad_norm_fp32([], norm_type=norm_type)` for `norm_type in [2.0, inf, 1.0]`
returns `0.0` without raising (regression test for #5529 / #5530)
- `get_grad_norm_fp32(grads, norm_type=norm_type)` for the same three
norm types on a small set of real gradients, checked against the
equivalent `torch.norm`-based computation

**Describe alternatives you've considered**

Could test only the empty-list case since that's what #5530 fixed, but
covering the non-empty path too gives a baseline so a future change to
any branch doesn't silently break the others.

**Additional context**

Follow-up to #5529 / #5530.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.