Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the tinker dependency to version 0.16.1 and refactors the model output processing logic in common.py to better handle pipeline parallel stages. Specifically, _tinker_build_output now correctly identifies and skips stages that produce no outputs, and _normalize_tensor_output has been updated to handle empty lists and tensor concatenation. Feedback was provided to improve memory efficiency by moving individual tensors to the CPU before concatenation to prevent potential GPU OOM issues.
There was a problem hiding this comment.
Pull request overview
Fixes tensor collection/normalization for Tinker-compatible server backends (especially when some pipeline stages produce no logits/logps), and updates the Docker image to use a newer tinker version.
Changes:
- Normalize
logits/logpsoutputs in a single path and treat empty lists as “missing output” to avoid downstream collection issues. - Return an empty per-datum output list when neither logits nor logps are produced (e.g., non-last PP stage).
- Bump
tinkerin the Dockerfile from0.14.0to0.16.1.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/twinkle/server/model/backends/common.py |
Adjusts output extraction/normalization to handle None and empty-list outputs cleanly for distributed/pipeline scenarios. |
Dockerfile |
Updates the container dependency pin for tinker to a newer version. |
PR type
PR information
Write the detail information belongs to this PR.
Experiment results
Paste your experiment result here(if needed).