test: direct unit suite for distillation losses and balancers (54 tests)#1924
test: direct unit suite for distillation losses and balancers (54 tests)#1924arham766 wants to merge 1 commit into
Conversation
Hand-computed exact values for the loss math (softmax/KL derivations in comments): LogitsDistillationLoss reductions and T^2 scaling, MFT corrected distributions for correct/incorrect argmax and the confident-correct one-hot, MGD alignment/mask/scale behavior, and StaticLossBalancer aggregation, validation, and grad preservation. The existing distill tests route losses through convert end-to-end with no numeric assertions (and are timm-blocked); this is the first value-level coverage. Adversarially reviewed: derivations re-computed independently, 3/3 seeded mutations killed. Documents five defects with NOTE tests or report: the deprecated F.kl_div mean default (semantics change in a future torch major), MGD not detaching teacher features (unlike its siblings), StaticLossBalancer crashing on int weights, individually negative weights passing validation, and the non-ABC abstractmethod on the balancer base. Part of the coverage initiative in NVIDIA#1902. Signed-off-by: arham766 <arhamislam766@yahoo.com>
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
1 participant
What does this PR do?
Type of change: new tests
Part of the unit-coverage initiative in #1902. First value-level coverage of modelopt/torch/distill/losses.py + loss_balancers.py — the existing distill tests route losses through convert end-to-end with zero numeric assertions (and are timm-blocked in minimal envs). Hand-computed exact values with derivations in comments: LogitsDistillationLoss (0.25·ln(4/3) for [0, ln3] vs uniform under mean; batchmean; exact T² scaling), MFT corrected distributions for correct/incorrect argmax and confident-correct one-hot (re-derived independently by the reviewer from the source formula), MGD align/mask/scale behavior, StaticLossBalancer aggregation (0.3·2+0.4·3+0.3·10=4.8), validation, and grad-graph preservation. Adversarial review: 3/3 seeded mutations killed. Five defects documented (NOTE tests / report; fixes offered): F.kl_div's deprecated "mean" default will silently rescale the default KD loss when torch flips semantics; MGDLoss does not detach teacher features (its siblings do — gradients flow into a standalone teacher); StaticLossBalancer(1) crashes on int weights; individually negative weights pass the sum-only validation; the balancer base declares abstractmethod without ABCMeta.
Usage
N/A — tests only.
Testing
Hermetic, CPU-only, deterministic, <1s. Full tests/unit/torch/quantization dir green alongside (789 passed, pre-existing skips only). Adversarially reviewed with independent re-derivation and mutation testing as described.
Before your PR is "Ready for review"
Additional Information
Issue: #1902