Repository navigation
fix series of failed test case for janus model #44739
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
Changes from all commits
f151716
7ca109f
0496db0
ef35abc
69b2b12
a7617a5
b0edb6d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,6 +35,7 @@ | |
| from transformers.models.auto.modeling_auto import MODEL_FOR_BACKBONE_MAPPING_NAMES, MODEL_MAPPING_NAMES | ||
| from transformers.testing_utils import ( | ||
| Expectations, | ||
| require_deterministic_for_xpu, | ||
| require_torch, | ||
| slow, | ||
| torch_device, | ||
|
|
@@ -199,6 +200,20 @@ class JanusVisionText2TextModelTest(ModelTesterMixin, GenerationTesterMixin, Pip | |
| ) | ||
| _is_composite = True | ||
|
|
||
| @staticmethod | ||
| def _prepare_config_headdim(config, requested_dim): | ||
| """ | ||
| Override to ensure vision_config.projection_dim stays in sync with text_config.hidden_size. | ||
| The aligner projects vision features to text embedding dimension, so they must match. | ||
| """ | ||
| from tests.test_modeling_common import ModelTesterMixin | ||
|
|
||
| config = ModelTesterMixin._prepare_config_headdim(config, requested_dim) | ||
| # Sync projection_dim with text hidden_size since aligner output must match text embeddings | ||
| if hasattr(config, "vision_config") and hasattr(config, "text_config"): | ||
| config.vision_config.projection_dim = config.text_config.hidden_size | ||
|
Comment on lines
+212
to
+214
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what's the error message for
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cc @vasqu to see if you have comments on this change.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems very model dependent on me. In this case, it seems like the text config got properly adjusted while the vision one stayed the same (and janus can't handle that). The whole purpose behind that one was for triton kernels (e.g. some hf kernels) to work because triton can't handle dims <16. So, likely a valid change - but I cannot tell if it would make sense in general, i.e. for other models.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Error message, hard to tell - likely some runtime error down the line because shapes didnt match anymore
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. W/o this change, it will return error:
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This is only in test_modeling_janus, not the general one in OK to have this change here after the above discussion |
||
| return config | ||
|
|
||
| def setUp(self): | ||
| self.model_tester = JanusVisionText2TextModelTester(self) | ||
| self.config_tester = ConfigTester(self, config_class=JanusConfig, has_text_modality=False) | ||
|
|
@@ -409,6 +424,7 @@ def setUp(self): | |
| self.model_id = "deepseek-community/Janus-Pro-1B" | ||
|
|
||
| @slow | ||
| @require_deterministic_for_xpu | ||
| def test_model_text_generation(self): | ||
| model = JanusForConditionalGeneration.from_pretrained(self.model_id, device_map="auto") | ||
| model.eval() | ||
|
|
@@ -428,6 +444,7 @@ def test_model_text_generation(self): | |
| ) | ||
|
|
||
| @slow | ||
| @require_deterministic_for_xpu | ||
| def test_model_text_generation_batched(self): | ||
| model = JanusForConditionalGeneration.from_pretrained(self.model_id, device_map="auto") | ||
| processor = AutoProcessor.from_pretrained(self.model_id) | ||
|
|
@@ -447,15 +464,25 @@ def test_model_text_generation_batched(self): | |
| images=[image_1, image_2], text=prompts, generation_mode="text", padding=True, return_tensors="pt" | ||
| ).to(model.device, torch.float16) | ||
|
|
||
| EXPECTED_TEXT_COMPLETION = [ | ||
| "You are a helpful language and vision assistant. You are able to understand the visual content that the user provides, and assist the user with a variety of tasks using natural language.\n\n\nDescribe what do you see here and tell me about the history behind it?\n\nThe image depicts the constellation of Leo, which is part of the zodiac and the constellation", # fmt: skip | ||
| "You are a helpful language and vision assistant. You are able to understand the visual content that the user provides, and assist the user with a variety of tasks using natural language.\n\nWhat constellation is this image showing?\nforming a constellation, the constellation of Orion is located in the constellation of Scorpius.\n", # fmt: skip | ||
| ] | ||
| EXPECTED_TEXT_COMPLETION = Expectations( | ||
| { | ||
| ("xpu", None): [ | ||
| "You are a helpful language and vision assistant. You are able to understand the visual content that the user provides, and assist the user with a variety of tasks using natural language.\n\n\nDescribe what do you see here and tell me about the history behind it?\n\nThe image depicts the constellation of Leo, which is part of the zodiac and the constellation", # fmt: skip | ||
| "You are a helpful language and vision assistant. You are able to understand the visual content that the user provides, and assist the user with a variety of tasks using natural language.\n\nWhat constellation is this image showing?\n\nThe image shows a constellation that is shaped like a stylized figure with a long tail. This", # fmt: skip | ||
| ], | ||
| (None, None): [ | ||
| "You are a helpful language and vision assistant. You are able to understand the visual content that the user provides, and assist the user with a variety of tasks using natural language.\n\n\nDescribe what do you see here and tell me about the history behind it?\n\nThe image depicts the constellation of Leo, which is part of the zodiac and is one", # fmt: skip | ||
| "You are a helpful language and vision assistant. You are able to understand the visual content that the user provides, and assist the user with a variety of tasks using natural language.\n\nWhat constellation is this image showing?\n\nThe image shows a constellation of a winged figure. This constellation is the **Luna**, also", # fmt: skip | ||
| ], | ||
| } | ||
| ) | ||
| generated_ids = model.generate(**inputs, max_new_tokens=20, generation_mode="text", do_sample=False) | ||
| text = processor.batch_decode(generated_ids, skip_special_tokens=True) | ||
| self.assertEqual(EXPECTED_TEXT_COMPLETION, text) | ||
| expected_text = EXPECTED_TEXT_COMPLETION.get_expectation() | ||
| self.assertEqual(expected_text, text) | ||
|
|
||
| @slow | ||
| @require_deterministic_for_xpu | ||
| def test_model_text_generation_with_multi_image(self): | ||
| model = JanusForConditionalGeneration.from_pretrained(self.model_id, device_map="auto") | ||
| processor = AutoProcessor.from_pretrained(self.model_id) | ||
|
|
@@ -472,12 +499,13 @@ def test_model_text_generation_with_multi_image(self): | |
| model.device, torch.float16 | ||
| ) | ||
|
|
||
| EXPECTED_TEXT_COMPLETION = ['You are a helpful language and vision assistant. You are able to understand the visual content that the user provides, and assist the user with a variety of tasks using natural language.\n\nWhat do these two images and have in common?\n\nThe two images you provided are of the same constellation. The first image shows the constellation of Leo, and the second image shows the constellation of Ursa Major. Both constellations are part of'] # fmt: skip | ||
| EXPECTED_TEXT_COMPLETION = ["You are a helpful language and vision assistant. You are able to understand the visual content that the user provides, and assist the user with a variety of tasks using natural language.\n\nWhat do these two images and have in common?\n\nThe two images you provided are of the same star constellation. The first image shows the constellation of Leo, and the second image shows the constellation of Ursa Major. Both constellations are part"] # fmt: skip | ||
| generated_ids = model.generate(**inputs, max_new_tokens=40, do_sample=False) | ||
| text = processor.batch_decode(generated_ids, skip_special_tokens=True) | ||
| self.assertEqual(EXPECTED_TEXT_COMPLETION, text) | ||
|
|
||
| @slow | ||
| @require_deterministic_for_xpu | ||
| def test_model_generate_images(self): | ||
| model = JanusForConditionalGeneration.from_pretrained(self.model_id, device_map="auto") | ||
| processor = AutoProcessor.from_pretrained(self.model_id) | ||
|
|
||
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.
Should not change this just for testing. Let's update the expected output in the test insteald
Uh oh!
There was an error while loading. Please reload this page.
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.
I recommend to keep the change here instead of in the test, just like in llama4, as users shouldn't need to know this implementation detail. The processor should work correctly out-of-the-box for the common use case (batched generation). WDYT?
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.
OK, let's see
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.
I have to run on Nvidia to see if I need to update anything in the test however