Add Tuna model support scaffolding#3315
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
d73caef to
43f2709
Compare
hsliuustc0106
left a comment
There was a problem hiding this comment.
BLOCKER scan:
| Category | Result |
|---|---|
| Correctness | PASS |
| Reliability/Safety | PASS |
| Breaking Changes | PASS |
| Test Coverage | PASS |
| Documentation | ISSUES |
| Security | PASS |
BLOCKING ISSUES:
-
[Documentation] Missing documentation updates: This PR adds Tuna model recognition scaffolding but does not update any documentation. Even for placeholder integration, the documentation should be updated to:
- Mention Tuna/Tuna-2 as a recognized model in
docs/models/supported_models.md(or equivalent) - Note the current status: recognized but runtime integration not yet available
- Link to the upstream repository (facebookresearch/tuna-2) for users who want to track progress
Without this documentation, users may be confused by the "not ready yet" error message.
- Mention Tuna/Tuna-2 as a recognized model in
Non-blocking suggestions:
-
Missing e2e validation of error message: The tests validate model detection and config resolution, but don't actually verify that the error message is actionable for users. Consider adding a test that instantiates
TunaExternalPipelineand validates the error message contains the key information users need (upstream repo reference, current status). -
Consider adding integration TODO: The error message is clear, but consider adding a TODO comment or issue reference in the code pointing to #3303 for future maintainers working on full Tuna integration.
-
Type annotation consistency: In
pipeline_tuna.py, theforwardandload_weightsmethods raise the same error but have different signatures. Consider whetherload_weightsshould accept*args, **kwargsfor consistency with other pipelines.
The scaffolding approach is reasonable given upstream constraints. Good unit test coverage for model detection.
Signed-off-by: Zeel <desaizeel2128@gmail.com>
43f2709 to
5b5ef48
Compare
|
@hsliuustc0106
|
|
Hey @zeel2104, the convention is to include the entire model in one single PR, and then you can ping code maintainers like |
|
@zhangj1an Thanks for the guidance! I’ll continue this PR as the full Tuna/Tuna-2 integration instead of only recognition scaffolding. Before I start porting the runtime path, could you please confirm the expected checkpoint/model artifact to validate against? Should I use the upstream Also, is it acceptable for the initial integration to depend on the upstream Tuna repo utilities, or should the required model/pipeline code be ported into Once I have that clarified, I’ll add actual inference support, offline examples, docs, tests, and visual outputs in the PR description similar to #2861. |
|
Hey @zeel2104, you should start with official model weights (but I have not seen facebook released it yet...), like models listed in The PR should be purely runnable on its own, without having to depend on upstream repo at all. If it used packages outside of |
Thanks @zhangj1an |
|
Thanks for your contribute! Let's keep this PR open for now. Once they release the weights, we can move forward. |
|
Hi @zeel2104, friendly reminder — this PR hasn't had any activity (commits or reviews) in the past 12 days. 🕐 Could you please provide an update?
Thanks for your contribution! 🙏 |
Purpose
Adds initial Tuna/Tuna-2 model support scaffolding for #3303.
This PR makes vLLM-Omni recognize Tuna/Tuna-2 model metadata and route it through a dedicated Tuna diffusion pipeline entrypoint. It includes:
TunaExternalPipelineregistry entry and placeholder pipeline.Note: this is not full end-to-end Tuna inference yet. Upstream
facebookresearch/tuna-2currently uses its own Hydra-based inference flow and.ptcheckpoint format, and full released weights / a stable HF-style loading contract are not available yet. The placeholder pipeline gives users a clear actionable error instead of an unknown-model failure.Test Plan
Run lint on touched files:
Run focused tests:
Test Result
Ruff passed:
Focused pytest passed in WSL with Python 3.12: