Skip to content

test: add coverage for compute_kl_divergence per_sample mode and disable_adapters - #24

Merged
marksverdhei merged 1 commit into
marksverdhei:mainfrom
hai-pilgrim:test/kl-per-sample-coverage
Mar 29, 2026
Merged

marksverdhei merged 1 commit into
marksverdhei:mainfrom
hai-pilgrim:test/kl-per-sample-coverage

Conversation

@hai-pilgrim

Copy link
Copy Markdown

Summary

Extends tests/test_kl.py from 6 → 13 tests, covering two previously untested code paths:

compute_kl_divergence(per_sample=True) — used in the vectorized loss loop (PR #20):

  • Returns a [batch] tensor (not a scalar)
  • Identical distributions → all zeros
  • mean(per_sample) equals the scalar result when masks are uniform
  • All per-sample values are non-negative
  • A fully-masked sample (all padding) contributes exactly 0

disable_adapters context manager — used in teacher forward passes:

  • Calls disable_adapter_layers() on entry and enable_adapter_layers() on exit
  • Re-enables adapters even when the body raises an exception (the critical correctness invariant)

Test plan

  • 13 tests, all pass
  • Full suite (excl. benchmark): 67 passed, 3 skipped

🤖 Generated with Claude Code

…le_adapters

Extends test_kl.py with coverage for two previously untested paths:

**per_sample=True** (5 tests):
- Returns [batch] tensor with correct shape
- Identical distributions → all zeros
- mean(per_sample) == scalar result when masks are uniform
- All values are non-negative
- Fully-masked sample contributes 0 loss

**disable_adapters** (2 tests):
- Calls disable_adapter_layers() on entry and enable_adapter_layers() on exit
- Re-enables adapters even when the body raises an exception

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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