llava_onevision

[chengtao-lv]

No description provided.

[gemini-code-assist]

Hello @chengtao-lv, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

Hello! Gemini here, providing a summary of this pull request to help everyone get up to speed quickly.

This pull request, titled "llava_onevision", introduces support for a new model variant, likely related to a "OneVision" version of LLaVA, into the llmc framework. The core changes involve adding a new Python file that implements this model and its evaluation wrapper, integrating it into the model registry, and making minor adjustments in other related modules for compatibility and robustness.

Highlights

• New Model Implementation: A new model class Llava_OneVision is added, inheriting from Llama, specifically designed to integrate a LLaVA 'OneVision' variant. This includes logic for loading the multimodal model, handling its configuration, and extracting vision-related components.
• Evaluation Integration: An evaluation wrapper class Llava_OneVision_Eval is introduced, inheriting from lmms_eval.models.llava_onevision.Llava_OneVision. This class sets up the model for evaluation using the lmms_eval framework, including handling device placement and distributed training setup via accelerate.
• Model Registry Update: The new Llava_OneVision model and its evaluation wrapper are registered in the llmc.utils.registry_factory.MODEL_REGISTRY, making them available for use within the framework.
• Robust Import Handling: A try...except block is added around the import of Qwen2VLVisionBlock in the token reduction module (tome.py) to gracefully handle cases where the required transformers version might not be installed, logging an informative message instead of crashing.
• Sparsification Config Fix: A minor correction is made in the base sparsification logic (base_blockwise_sparsification.py) to correctly access the 'n_prune_layers' key within the 'weight' section of the sparsity configuration.

Changelog

Click here to see the changelog

• llmc/compression/sparsification/base_blockwise_sparsification.py
  • Corrected access to 'n_prune_layers' key within the 'weight' sparsity configuration (line 43).
• llmc/compression/token_reduction/tome.py
  • Added loguru.logger import (line 7).
  • Wrapped Qwen2VLVisionBlock import in a try...except ModuleNotFoundError block for graceful failure if the class is not available (lines 10-15).
• llmc/models/init.py
  • Imported the new Llava_OneVision model (line 14).
• llmc/models/llava_onevision.py
  • Added a new file implementing the Llava_OneVision model and Llava_OneVision_Eval evaluation wrapper.
  • Implemented model loading using load_pretrained_model from llava.model.builder.
  • Configured model parameters including attention implementation based on PyTorch version.
  • Integrated with lmms_eval for evaluation setup.
  • Handled distributed training setup using accelerate.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

---

A new model arrives,
Vision and text now thrives,
Code flows with grace,
In this digital space,
Through diffs, the engineer strives.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces support for the Llava_OneVision model, including its definition and evaluation setup. The changes also incorporate a logical correction in base_blockwise_sparsification.py and robust import handling in tome.py. The new model implementation in llava_onevision.py is the main focus. Overall, the additions are clear, but there are opportunities to enhance flexibility, reduce potential side effects, and improve documentation, as detailed in the specific comments.

Summary of Findings

• Hardcoded Configuration Values: The Llava_OneVision class in llava_onevision.py hardcodes model_name and overwrite_config values. This might limit its flexibility for different LLaVA model variants. Consider making these configurable or deriving them dynamically.
• Side Effects on Input Objects: The Llava_OneVision_Eval class in llava_onevision.py modifies its input llmc_model by deleting attributes. This side effect can be surprising and may lead to issues if the llmc_model instance is used elsewhere. It's generally better to avoid modifying input objects unless explicitly documented and necessary.
• Missing Docstrings: The new classes and their primary methods in llava_onevision.py lack docstrings, which are important for understanding and maintaining the code. Adding them is recommended for improved clarity.
• Improved Configuration Access in Sparsification: The change in llmc/compression/sparsification/base_blockwise_sparsification.py correctly refines how n_prune_layers is accessed from the sparsity configuration, making it more consistent and robust.
• Robust Optional Dependency Handling: The try-except block added in llmc/compression/token_reduction/tome.py for Qwen2VLVisionBlock import is a good practice for handling optional dependencies gracefully.

Merge Readiness

While the core functionality for Llava_OneVision is well-implemented, there are a few areas identified with medium severity that could be improved for better flexibility, clarity, and maintainability, particularly regarding hardcoded configurations, side effects on input objects, and missing docstrings. I recommend addressing these points before merging. As an AI, I am not authorized to approve pull requests; please ensure further review and approval from the team.

[gemini-code-assist]

The model_name is hardcoded to 'llava_qwen' (line 56) and overwrite_config (lines 58-60) also contains hardcoded values (e.g., mm_spatial_pool_stride = 2). Could these fixed values limit the flexibility of Llava_OneVision if it needs to support other LLaVA variants or configurations derived from self.model_path? Perhaps these could be made configurable or dynamically determined?

[gemini-code-assist]

Deleting attributes (tokenizer, image_processor, max_length) from the passed llmc_model instance (lines 171-173) introduces a side effect by modifying the input object. This can be unexpected for the caller and might lead to issues if llmc_model is used elsewhere after this call. Is this modification strictly necessary? If these components are needed by Llava_OneVision_Eval, could they be accessed via llmc_model when required, or if copies are made, could the original llmc_model be left unchanged?

[gemini-code-assist]

The new class Llava_OneVision (line 39) and its primary methods like build_model (line 43) currently lack docstrings. Adding them would significantly improve code understanding and maintainability by explaining their purpose, parameters, and how they function. Could you please add these?

[gemini-code-assist]

Similarly, the new class Llava_OneVision_Eval (line 110) and its __init__ method (line 113) would benefit greatly from docstrings. Documenting their roles, especially the parameters for __init__, would make the code easier to use and maintain. Could these be added?