fix: warn when dataset samples get truncated past max_seq_length - #426
SahilKumar75 merged 2 commits into
Conversation
dataset.py was silently cutting off any sample longer than max_seq_length, no way to know it happened unless you went digging manually. now it measures the real token length before truncating, and logs a warning with the percentage of samples affected and the average tokens lost once tokenization finishes. added tests for the stats helper and for load_and_tokenize actually logging it with a fake tokenizer. fixes SahilKumar75#181
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Automated Review Checklist
Before approving this PR, confirm each item:
Code quality
- Logic is correct and edge cases are handled
- No debug logs, dead code, or commented-out blocks left in
- Naming is clear — no abbreviations that need a comment to decode
Tests
- New behaviour is covered by tests, or existing tests updated
-
poetry run pytestpasses locally
Frontend (if applicable)
- UI renders correctly across light/dark mode
- No layout regressions on narrow viewports
Infrastructure / config (if applicable)
- Secrets/env vars are not hardcoded
- Docker build still passes (
docker build .)
Docs
- CHANGELOG or PR description explains the why, not just the what
- Public API changes are reflected in docs
Review, check off what applies, then submit your formal Approve or Request Changes.
SahilKumar75
left a comment
There was a problem hiding this comment.
Reviewed the truncation-warning implementation and the cache-safety follow-up. Full tests, lint, formatting, and GitHub CI pass.
Summary
dataset.py was truncating any sample that went over max_seq_length without telling anyone. now it logs a warning after tokenizing, saying what percentage of samples got cut and by how much on average.
Scope
_summarize_truncationhelper and wired it intoload_and_tokenize, so it measures each sample's real token length before truncating and logs a warning if anything exceededmax_seq_length.Product Impact
anyone fine-tuning on a dataset with long examples was losing content silently, no way to tell without manually checking token lengths themselves. now they get a heads up in the logs, e.g. "12.5% of samples (5/40) exceeded max_seq_length=512 and were truncated (avg 34.2 tokens lost per truncated sample)", so they know to either bump max_seq_length or trim their data.
Technical Notes
load_and_tokenizebatches through.map(), so I collect lengths in a list declared outside the batch function (closure) and extend it on every batch call, then run the stats + log once after.map()finishes with the full dataset's lengths. required an extratruncation=Falsetokenizer call per batch just to get the untruncated length, since the real encode call already truncates before you can measure it.Validation
ran ruff format, ruff check, and mypy on the changed files, all clean. full trainer/dataset.py test file passes (7/7).
Screenshots Or Recordings
n/a, backend-only logging change.
Risk And Rollback
low risk, purely additive (a log line), doesn't change what gets fed to the trainer. if it turns out to be noisy or wrong, reverting this one file is enough.