Migrate InternVLImagePixelInputs (in nemotron_vl.py) to TensorSchema #22023
Conversation
Signed-off-by: Benji Beck <[email protected]>
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add 🚀 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a great improvement. It successfully migrates InternVLImagePixelInputs in nemotron_vl.py to use TensorSchema for input validation, which is consistent with the pattern used in other multi-modal models like Phi3VImagePixelInputs.
The removal of the manual _validate_pixel_values method and the adoption of a declarative TensorSchema with resolve_bindings cleans up the code, enhances maintainability, and improves input contract enforcement. The changes are clear, focused, and well-aligned with the broader goal of standardizing input validation across models.
I've reviewed the changes and they look correct and robust. I don't have any concerns. Well done!
|
Hi @bbeckca, I see you are making a lot of individual small PRs for this refactor. Would you be able to consolidate them into fewer PRs to aid review and keep CI cost down? |
Sorry I’m late to seeing this. That makes sense. Is there a number of models per diff you’d recommend? |
cc @DarkLight1337 @Isotr0py for any additional thoughts. |
DarkLight1337
added
the
ready
|
Let's see if the test can pass |
Observing some failures due to |
|
You can merge after #23118 |
…llm-project#22023) Signed-off-by: Benji Beck <[email protected]> Co-authored-by: Cyrus Leung <[email protected]>
…llm-project#22023) Signed-off-by: Benji Beck <[email protected]> Co-authored-by: Cyrus Leung <[email protected]>
…llm-project#22023) Signed-off-by: Benji Beck <[email protected]> Co-authored-by: Cyrus Leung <[email protected]>
…llm-project#22023) Signed-off-by: Benji Beck <[email protected]> Co-authored-by: Cyrus Leung <[email protected]> Signed-off-by: Duncan Moss <[email protected]>
…llm-project#22023) Signed-off-by: Benji Beck <[email protected]> Co-authored-by: Cyrus Leung <[email protected]>
…llm-project#22023) Signed-off-by: Benji Beck <[email protected]> Co-authored-by: Cyrus Leung <[email protected]> Signed-off-by: Xiao Yu <[email protected]>
…llm-project#22023) Signed-off-by: Benji Beck <[email protected]> Co-authored-by: Cyrus Leung <[email protected]>
…llm-project#22023) Signed-off-by: Benji Beck <[email protected]> Co-authored-by: Cyrus Leung <[email protected]>
…llm-project#22023) Signed-off-by: Benji Beck <[email protected]> Co-authored-by: Cyrus Leung <[email protected]>
4 participants
Purpose
This PR migrates InternVLImagePixelInputs (in nemotron_vl.py) from a TypedDict-based definition to a structured TensorSchema model with runtime shape validation. This brings it in line with recent changes to Phi3VImagePixelInputs, and is part of a broader effort to improve input contract enforcement and debug-ability across multi-modal models.
Test Plan
Confirm validation works via standalone tests in tests/standalone_test/test_tensor_schema.py and rely on CI to check integration.
Test Result