Skip to content

fix: divide by length 1 instead of 1e-16 - #380

Merged
stephantul merged 1 commit into
mainfrom
fix-empty-text-nan
Sep 28, 2026
Merged

stephantul merged 1 commit into
mainfrom
fix-empty-text-nan

Conversation

@stephantul

Copy link
Copy Markdown
Contributor

Mean pooling divided by the number of tokens plus 1e-16. For a text without any tokens, the backward pass divided the gradient by 1e-16 after normalization. This overflows to inf and turned into NaN in the token weight gradient. Clamp the length to at least 1 instead: non-empty texts are unaffected, and empty texts get a zero embedding and a zero gradient.

Mean pooling divided by the number of tokens plus 1e-16. For a text without
any tokens, the backward pass divided the gradient by 1e-16 after normalize
had already scaled it up, which overflowed to inf and turned into NaN in the
token weight gradient. Clamp the length to at least 1 instead: non-empty
texts are unaffected, and empty texts get a zero embedding and a zero
gradient.
@stephantul
stephantul marked this pull request as ready for review September 27, 2026 17:46
@stephantul
stephantul requested a review from Pringled September 27, 2026 17:46
@codecov

codecov Bot commented Sep 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
model2vec/train/base.py 99.54% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@greptile-apps

greptile-apps Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Changes how the model handles empty token sequences.

The PR appears safe to merge; no actionable regression was identified.

Reviews (1) · Last reviewed commit: "fix(train): avoid NaN gradients for text..."

@stephantul
stephantul merged commit 059b835 into main Sep 28, 2026
12 checks passed
@stephantul
stephantul deleted the fix-empty-text-nan branch September 28, 2026 06:51
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