Conversation
koriyoshi2041
force-pushed
the
fix/llava-vision-hook-aliases
branch
from
September 26, 2026 02:42
b8f7332 to
52b6a9c
Compare
jlarson4
reviewed
Sep 28, 2026
jlarson4
left a comment
Collaborator
There was a problem hiding this comment.
Hi @koriyoshi2041! Thanks for closing the CLIP half of this gap. A couple small items to address below before we can merge.
Collaborator
|
Looks good! Approved |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The LLaVA-family adapters declare attention and MLP hook aliases for CLIP vision encoder layers, but
CLIPVisionEncoderLayerBridgedoes not register the corresponding submodules. The four aliases therefore cannot resolve.Fix
Register the CLIP layer norms, q/k/v/output projections, and fc1/fc2 MLP projections with the same generalized bridge structure used by the SigLIP tower. The attention bridge receives the vision tower dimensions rather than the language-model dimensions. The now-obsolete strict xfails are removed for LLaVA, LLaVA-Next, and LLaVA-OneVision.
Test
uv run pytest tests/unit/model_bridge/test_hook_alias_resolution.py tests/unit/model_bridge/supported_architectures/test_llava_adapter.py tests/unit/model_bridge/supported_architectures/test_llava_next_adapter.py tests/unit/model_bridge/supported_architectures/test_llava_onevision_adapter.py -q(192 passed, 1 skipped, 3 unrelated xfailed)make formatuv run mypy transformer_lens/model_bridge/generalized_components/clip_vision_encoder.pygit diff --checkRisk
The change is limited to CLIP vision-layer component registration and alias resolution. It does not change the wrapped Hugging Face forward path or claim end-to-end multimodal parity.