-
-
Notifications
You must be signed in to change notification settings - Fork 10.3k
[Model] Add Eagle 2.5 VL #24019
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[Model] Add Eagle 2.5 VL #24019
Conversation
Warning Gemini encountered an error creating the review. You can try again by commenting |
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds support for the Eagle 2.5 VL model. The changes include the model implementation, registration, and test configurations. The implementation appears to be largely adapted from existing models, but there are several critical copy-paste errors, particularly in handling video embeddings, which could lead to runtime failures. Additionally, there are multiple instances of misleading comments and docstrings that should be corrected for better maintainability. I've provided specific comments and suggestions to address these issues.
👋 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 You ask your reviewers to trigger select CI tests on top of 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 If you have any questions, please reach out to us on Slack at https://slack.vllm.ai. 🚀 |
max_transformers_version="4.48", # noqa: E501 | ||
transformers_version_reason="HF model is not compatible.", # noqa: E501 | ||
hf_overrides={"architectures": ["DeepseekVLV2ForCausalLM"]}), # noqa: E501 | ||
"Eagle2_5_VLForConditionalGeneration": _HfExamplesInfo("nvidia/Eagle2.5-8B"), # noqa: E501 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also add processor test in tests/models/multimodal/processing/test_common.py
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation of Eagle 2.5VL on HF still appears to have some compatibility issues. We will add the processor test after fixing the code on HF.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eagle 2.5 VL Processor has been included in the test. We wrap the HF version of Eagle 2.5 VL Processor to make it compatible with the vllm test, so that the tests can directly test the HF version of Eagle 2.5 VL Processor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tests/models/multimodal/processing/test_common.py
is used to make sure multimodal processor can work well with mm cache, which is unlikely to be covered by generation test.
Co-authored-by: Isotr0py <[email protected]> Signed-off-by: Lu Lidong <[email protected]>
Co-authored-by: Isotr0py <[email protected]> Signed-off-by: Lu Lidong <[email protected]>
Co-authored-by: Isotr0py <[email protected]> Signed-off-by: Lu Lidong <[email protected]>
Co-authored-by: Isotr0py <[email protected]> Signed-off-by: Lu Lidong <[email protected]>
Co-authored-by: Isotr0py <[email protected]> Signed-off-by: Lu Lidong <[email protected]>
Co-authored-by: Isotr0py <[email protected]> Signed-off-by: Lu Lidong <[email protected]>
Co-authored-by: Isotr0py <[email protected]> Signed-off-by: Lu Lidong <[email protected]>
Co-authored-by: Isotr0py <[email protected]> Signed-off-by: Lu Lidong <[email protected]>
Co-authored-by: Isotr0py <[email protected]> Signed-off-by: Lu Lidong <[email protected]>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Lu Lidong <[email protected]>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Lu Lidong <[email protected]>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Lu Lidong <[email protected]>
…e with vllm tests, so that tests can directly test the HF version of Eagle 2.5 VL Processor
when will this PR be reviewed and merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the dealy! Just leave some minor comments. PTAL!
max_transformers_version="4.48", # noqa: E501 | ||
transformers_version_reason="HF model is not compatible.", # noqa: E501 | ||
hf_overrides={"architectures": ["DeepseekVLV2ForCausalLM"]}), # noqa: E501 | ||
"Eagle2_5_VLForConditionalGeneration": _HfExamplesInfo("nvidia/Eagle2.5-8B"), # noqa: E501 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tests/models/multimodal/processing/test_common.py
is used to make sure multimodal processor can work well with mm cache, which is unlikely to be covered by generation test.
Co-authored-by: Isotr0py <[email protected]> Signed-off-by: Lu Lidong <[email protected]>
Co-authored-by: Isotr0py <[email protected]> Signed-off-by: Lu Lidong <[email protected]>
This pull request has merge conflicts that must be resolved before it can be |
Purpose
This PR implements support for the Eagle 2.5 VL model by NVIDIA.
Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.md
andexamples
for a new model.