From 1ea32585fbe438013b35f343b28f5250969e2a0c Mon Sep 17 00:00:00 2001 From: raushan Date: Fri, 13 Sep 2024 09:52:55 +0200 Subject: [PATCH 1/7] fix --- src/transformers/processing_utils.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/transformers/processing_utils.py b/src/transformers/processing_utils.py index ee28c01189b4..1d86b128d31d 100644 --- a/src/transformers/processing_utils.py +++ b/src/transformers/processing_utils.py @@ -502,9 +502,12 @@ def save_pretrained(self, save_directory, push_to_hub: bool = False, **kwargs): output_chat_template_file = os.path.join(save_directory, CHAT_TEMPLATE_NAME) processor_dict = self.to_dict() - chat_template = processor_dict.pop("chat_template", None) - if chat_template is not None: - chat_template_json_string = json.dumps({"chat_template": chat_template}, indent=2, sort_keys=True) + "\n" + # Save `chat_template` in its own file. We can't get it from `processor_dict` as we popped it in `to_dict` + # to avoid serializing chat template in json config file. So let's get it from `self` directly + if self.chat_template is not None: + chat_template_json_string = ( + json.dumps({"chat_template": self.chat_template}, indent=2, sort_keys=True) + "\n" + ) with open(output_chat_template_file, "w", encoding="utf-8") as writer: writer.write(chat_template_json_string) logger.info(f"chat template saved in {output_chat_template_file}") From f38e6cbbb533a0f626f014c92329ef3ff935abf6 Mon Sep 17 00:00:00 2001 From: raushan Date: Fri, 13 Sep 2024 10:33:04 +0200 Subject: [PATCH 2/7] add tests --- tests/models/llava/test_processor_llava.py | 44 +++++++++++++++++- .../llava_next/test_processor_llava_next.py | 46 ++++++++++++++++++- .../test_processing_llava_onevision.py | 23 +++++++++- 3 files changed, 108 insertions(+), 5 deletions(-) diff --git a/tests/models/llava/test_processor_llava.py b/tests/models/llava/test_processor_llava.py index 54c1b4674cbc..d92d32a96042 100644 --- a/tests/models/llava/test_processor_llava.py +++ b/tests/models/llava/test_processor_llava.py @@ -11,18 +11,58 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import json +import tempfile import unittest +from transformers import AutoTokenizer, LlamaTokenizerFast, LlavaProcessor from transformers.testing_utils import require_vision from transformers.utils import is_vision_available +from ...test_processing_common import ProcessorTesterMixin + if is_vision_available(): - from transformers import AutoTokenizer, LlavaProcessor + from transformers import CLIPImageProcessor @require_vision -class LlavaProcessorTest(unittest.TestCase): +class LlavaProcessorTest(ProcessorTesterMixin, unittest.TestCase): + processor_class = LlavaProcessor + + def setUp(self): + self.tmpdirname = tempfile.mkdtemp() + + image_processor = CLIPImageProcessor() + tokenizer = LlamaTokenizerFast.from_pretrained("huggyllama/llama-7b") + processor_kwargs = self.prepare_processor_dict() + processor = LlavaProcessor(image_processor, tokenizer, **processor_kwargs) + processor.save_pretrained(self.tmpdirname) + + def get_tokenizer(self, **kwargs): + return LlavaProcessor.from_pretrained(self.tmpdirname, **kwargs).tokenizer + + def get_image_processor(self, **kwargs): + return LlavaProcessor.from_pretrained(self.tmpdirname, **kwargs).image_processor + + def prepare_processor_dict(self): + return {"chat_template": "dummy_template"} + + def test_processor_to_json_string(self): + processor = self.get_processor() + obj = json.loads(processor.to_json_string()) + for key, value in self.prepare_processor_dict().items(): + # chat templates are popped from dict + if key != "chat_template": + self.assertEqual(obj[key], value) + self.assertEqual(getattr(processor, key, None), value) + + def test_chat_template_is_saved(self): + with tempfile.TemporaryDirectory() as tmpdirname: + processor_loaded = self.processor_class.from_pretrained(tmpdirname) + processor_dict = self.prepare_processor_dict() + self.assertTrue(processor_loaded.chat_template == processor_dict.get("chat_template", None)) + def test_can_load_various_tokenizers(self): for checkpoint in ["Intel/llava-gemma-2b", "llava-hf/llava-1.5-7b-hf"]: processor = LlavaProcessor.from_pretrained(checkpoint) diff --git a/tests/models/llava_next/test_processor_llava_next.py b/tests/models/llava_next/test_processor_llava_next.py index c8b58ce7982f..12695c4deb5b 100644 --- a/tests/models/llava_next/test_processor_llava_next.py +++ b/tests/models/llava_next/test_processor_llava_next.py @@ -11,20 +11,62 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import json +import tempfile import unittest import torch +from transformers import AutoProcessor, LlamaTokenizerFast, LlavaNextProcessor from transformers.testing_utils import require_vision from transformers.utils import is_vision_available +from ...test_processing_common import ProcessorTesterMixin + if is_vision_available(): - from transformers import AutoProcessor + from transformers import CLIPImageProcessor @require_vision -class LlavaProcessorTest(unittest.TestCase): +class LlavaNextProcessorTest(ProcessorTesterMixin, unittest.TestCase): + processor_class = LlavaNextProcessor + + def setUp(self): + self.tmpdirname = tempfile.mkdtemp() + + image_processor = CLIPImageProcessor() + tokenizer = LlamaTokenizerFast.from_pretrained("huggyllama/llama-7b") + processor_kwargs = self.prepare_processor_dict() + processor = LlavaNextProcessor(image_processor, tokenizer, **processor_kwargs) + processor.save_pretrained(self.tmpdirname) + + def get_tokenizer(self, **kwargs): + return LlavaNextProcessor.from_pretrained(self.tmpdirname, **kwargs).tokenizer + + def get_image_processor(self, **kwargs): + return LlavaNextProcessor.from_pretrained(self.tmpdirname, **kwargs).image_processor + + def prepare_processor_dict(self): + return {"chat_template": "dummy_template"} + + # Copied from tests.models.llava.test_processor_llava.LlavaProcessorTest.test_processor_to_json_string + def test_processor_to_json_string(self): + processor = self.get_processor() + obj = json.loads(processor.to_json_string()) + for key, value in self.prepare_processor_dict().items(): + # chat templates are popped from dict + if key != "chat_template": + self.assertEqual(obj[key], value) + self.assertEqual(getattr(processor, key, None), value) + + # Copied from tests.models.llava.test_processor_llava.LlavaProcessorTest.test_chat_template_is_saved + def test_chat_template_is_saved(self): + with tempfile.TemporaryDirectory() as tmpdirname: + processor_loaded = self.processor_class.from_pretrained(tmpdirname) + processor_dict = self.prepare_processor_dict() + self.assertTrue(processor_loaded.chat_template == processor_dict.get("chat_template", None)) + def test_chat_template(self): processor = AutoProcessor.from_pretrained("llava-hf/llava-v1.6-vicuna-7b-hf") expected_prompt = "USER: \nWhat is shown in this image? ASSISTANT:" diff --git a/tests/models/llava_onevision/test_processing_llava_onevision.py b/tests/models/llava_onevision/test_processing_llava_onevision.py index e045f2ba7f0b..39affa0b0948 100644 --- a/tests/models/llava_onevision/test_processing_llava_onevision.py +++ b/tests/models/llava_onevision/test_processing_llava_onevision.py @@ -11,6 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import json import shutil import tempfile import unittest @@ -52,9 +53,29 @@ def get_tokenizer(self, **kwargs): def get_image_processor(self, **kwargs): return AutoProcessor.from_pretrained(self.tmpdirname, **kwargs).image_processor - def get_Video_processor(self, **kwargs): + def get_video_processor(self, **kwargs): return AutoProcessor.from_pretrained(self.tmpdirname, **kwargs).video_processor + def prepare_processor_dict(self): + return {"chat_template": "dummy_template"} + + # Copied from tests.models.llava.test_processor_llava.LlavaProcessorTest.test_processor_to_json_string + def test_processor_to_json_string(self): + processor = self.get_processor() + obj = json.loads(processor.to_json_string()) + for key, value in self.prepare_processor_dict().items(): + # chat templates are popped from dict + if key != "chat_template": + self.assertEqual(obj[key], value) + self.assertEqual(getattr(processor, key, None), value) + + # Copied from tests.models.llava.test_processor_llava.LlavaProcessorTest.test_chat_template_is_saved + def test_chat_template_is_saved(self): + with tempfile.TemporaryDirectory() as tmpdirname: + processor_loaded = self.processor_class.from_pretrained(tmpdirname) + processor_dict = self.prepare_processor_dict() + self.assertTrue(processor_loaded.chat_template == processor_dict.get("chat_template", None)) + def tearDown(self): shutil.rmtree(self.tmpdirname) From bc40e0e13d76d8b5aabbe4a5206b43a92577894d Mon Sep 17 00:00:00 2001 From: raushan Date: Fri, 13 Sep 2024 11:14:45 +0200 Subject: [PATCH 3/7] fix tests --- tests/models/llava/test_processor_llava.py | 7 +++---- tests/models/llava_next/test_processor_llava_next.py | 7 +++---- .../llava_onevision/test_processing_llava_onevision.py | 10 +++++----- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/tests/models/llava/test_processor_llava.py b/tests/models/llava/test_processor_llava.py index d92d32a96042..095f4ac5ece0 100644 --- a/tests/models/llava/test_processor_llava.py +++ b/tests/models/llava/test_processor_llava.py @@ -58,10 +58,9 @@ def test_processor_to_json_string(self): self.assertEqual(getattr(processor, key, None), value) def test_chat_template_is_saved(self): - with tempfile.TemporaryDirectory() as tmpdirname: - processor_loaded = self.processor_class.from_pretrained(tmpdirname) - processor_dict = self.prepare_processor_dict() - self.assertTrue(processor_loaded.chat_template == processor_dict.get("chat_template", None)) + processor_loaded = self.processor_class.from_pretrained(self.tmpdirname) + processor_dict = self.prepare_processor_dict() + self.assertTrue(processor_loaded.chat_template == processor_dict.get("chat_template", None)) def test_can_load_various_tokenizers(self): for checkpoint in ["Intel/llava-gemma-2b", "llava-hf/llava-1.5-7b-hf"]: diff --git a/tests/models/llava_next/test_processor_llava_next.py b/tests/models/llava_next/test_processor_llava_next.py index 12695c4deb5b..aba127ffe3c7 100644 --- a/tests/models/llava_next/test_processor_llava_next.py +++ b/tests/models/llava_next/test_processor_llava_next.py @@ -62,10 +62,9 @@ def test_processor_to_json_string(self): # Copied from tests.models.llava.test_processor_llava.LlavaProcessorTest.test_chat_template_is_saved def test_chat_template_is_saved(self): - with tempfile.TemporaryDirectory() as tmpdirname: - processor_loaded = self.processor_class.from_pretrained(tmpdirname) - processor_dict = self.prepare_processor_dict() - self.assertTrue(processor_loaded.chat_template == processor_dict.get("chat_template", None)) + processor_loaded = self.processor_class.from_pretrained(self.tmpdirname) + processor_dict = self.prepare_processor_dict() + self.assertTrue(processor_loaded.chat_template == processor_dict.get("chat_template", None)) def test_chat_template(self): processor = AutoProcessor.from_pretrained("llava-hf/llava-v1.6-vicuna-7b-hf") diff --git a/tests/models/llava_onevision/test_processing_llava_onevision.py b/tests/models/llava_onevision/test_processing_llava_onevision.py index 39affa0b0948..0cfc0d46bbf8 100644 --- a/tests/models/llava_onevision/test_processing_llava_onevision.py +++ b/tests/models/llava_onevision/test_processing_llava_onevision.py @@ -41,9 +41,10 @@ def setUp(self): image_processor = LlavaOnevisionImageProcessor() video_processor = LlavaOnevisionVideoProcessor() tokenizer = Qwen2TokenizerFast.from_pretrained("Qwen/Qwen2-0.5B-Instruct") + processor_kwargs = self.prepare_processor_dict() processor = LlavaOnevisionProcessor( - video_processor=video_processor, image_processor=image_processor, tokenizer=tokenizer + video_processor=video_processor, image_processor=image_processor, tokenizer=tokenizer, **processor_kwargs ) processor.save_pretrained(self.tmpdirname) @@ -71,10 +72,9 @@ def test_processor_to_json_string(self): # Copied from tests.models.llava.test_processor_llava.LlavaProcessorTest.test_chat_template_is_saved def test_chat_template_is_saved(self): - with tempfile.TemporaryDirectory() as tmpdirname: - processor_loaded = self.processor_class.from_pretrained(tmpdirname) - processor_dict = self.prepare_processor_dict() - self.assertTrue(processor_loaded.chat_template == processor_dict.get("chat_template", None)) + processor_loaded = self.processor_class.from_pretrained(self.tmpdirname) + processor_dict = self.prepare_processor_dict() + self.assertTrue(processor_loaded.chat_template == processor_dict.get("chat_template", None)) def tearDown(self): shutil.rmtree(self.tmpdirname) From 512930a297106c967532bc761eeb3d2cd5d878bf Mon Sep 17 00:00:00 2001 From: Raushan Turganbay Date: Mon, 16 Sep 2024 13:36:38 +0200 Subject: [PATCH 4/7] Update tests/models/llava/test_processor_llava.py Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> --- tests/models/llava/test_processor_llava.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/models/llava/test_processor_llava.py b/tests/models/llava/test_processor_llava.py index 095f4ac5ece0..e7cba299f910 100644 --- a/tests/models/llava/test_processor_llava.py +++ b/tests/models/llava/test_processor_llava.py @@ -53,9 +53,9 @@ def test_processor_to_json_string(self): obj = json.loads(processor.to_json_string()) for key, value in self.prepare_processor_dict().items(): # chat templates are popped from dict - if key != "chat_template": - self.assertEqual(obj[key], value) - self.assertEqual(getattr(processor, key, None), value) + self.assertFalse(key == "chat_template") + self.assertEqual(obj[key], value) + self.assertEqual(getattr(processor, key, None), value) def test_chat_template_is_saved(self): processor_loaded = self.processor_class.from_pretrained(self.tmpdirname) From 7a0b558a183a9b044160520e544633bc890c7d34 Mon Sep 17 00:00:00 2001 From: raushan Date: Mon, 16 Sep 2024 14:32:45 +0200 Subject: [PATCH 5/7] fix --- tests/models/llava/test_processor_llava.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/models/llava/test_processor_llava.py b/tests/models/llava/test_processor_llava.py index e7cba299f910..095f4ac5ece0 100644 --- a/tests/models/llava/test_processor_llava.py +++ b/tests/models/llava/test_processor_llava.py @@ -53,9 +53,9 @@ def test_processor_to_json_string(self): obj = json.loads(processor.to_json_string()) for key, value in self.prepare_processor_dict().items(): # chat templates are popped from dict - self.assertFalse(key == "chat_template") - self.assertEqual(obj[key], value) - self.assertEqual(getattr(processor, key, None), value) + if key != "chat_template": + self.assertEqual(obj[key], value) + self.assertEqual(getattr(processor, key, None), value) def test_chat_template_is_saved(self): processor_loaded = self.processor_class.from_pretrained(self.tmpdirname) From ce0f6e9dfb5fc563f3162871af6df1941418a7eb Mon Sep 17 00:00:00 2001 From: raushan Date: Tue, 17 Sep 2024 10:39:50 +0200 Subject: [PATCH 6/7] fix tests --- tests/models/llava/test_processor_llava.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/models/llava/test_processor_llava.py b/tests/models/llava/test_processor_llava.py index b6c80eab8d15..7f3d48dd3644 100644 --- a/tests/models/llava/test_processor_llava.py +++ b/tests/models/llava/test_processor_llava.py @@ -34,7 +34,7 @@ class LlavaProcessorTest(ProcessorTesterMixin, unittest.TestCase): def setUp(self): self.tmpdirname = tempfile.mkdtemp() - image_processor = CLIPImageProcessor() + image_processor = CLIPImageProcessor(do_center_crop=False) tokenizer = LlamaTokenizerFast.from_pretrained("huggyllama/llama-7b") processor_kwargs = self.prepare_processor_dict() processor = LlavaProcessor(image_processor, tokenizer, **processor_kwargs) From b681d9c92635762c6bffb2f9c23e46277099ab0f Mon Sep 17 00:00:00 2001 From: raushan Date: Tue, 17 Sep 2024 13:47:07 +0200 Subject: [PATCH 7/7] update tests --- tests/models/llava/test_processor_llava.py | 19 +++++++++++------- .../llava_next/test_processor_llava_next.py | 20 +++++++++++-------- .../test_processing_llava_onevision.py | 20 +++++++++++-------- 3 files changed, 36 insertions(+), 23 deletions(-) diff --git a/tests/models/llava/test_processor_llava.py b/tests/models/llava/test_processor_llava.py index 7f3d48dd3644..e62769e34509 100644 --- a/tests/models/llava/test_processor_llava.py +++ b/tests/models/llava/test_processor_llava.py @@ -52,17 +52,22 @@ def tearDown(self): def prepare_processor_dict(self): return {"chat_template": "dummy_template"} + @unittest.skip( + "Skip because the model has no processor kwargs except for chat template and" + "chat template is saved as a separate file. Stop skipping this test when the processor" + "has new kwargs saved in config file." + ) def test_processor_to_json_string(self): - processor = self.get_processor() - obj = json.loads(processor.to_json_string()) - for key, value in self.prepare_processor_dict().items(): - # chat templates are popped from dict - if key != "chat_template": - self.assertEqual(obj[key], value) - self.assertEqual(getattr(processor, key, None), value) + pass def test_chat_template_is_saved(self): processor_loaded = self.processor_class.from_pretrained(self.tmpdirname) + processor_dict_loaded = json.loads(processor_loaded.to_json_string()) + # chat templates aren't serialized to json in processors + self.assertFalse("chat_template" in processor_dict_loaded.keys()) + + # they have to be saved as separate file and loaded back from that file + # so we check if the same template is loaded processor_dict = self.prepare_processor_dict() self.assertTrue(processor_loaded.chat_template == processor_dict.get("chat_template", None)) diff --git a/tests/models/llava_next/test_processor_llava_next.py b/tests/models/llava_next/test_processor_llava_next.py index aba127ffe3c7..450034f4151d 100644 --- a/tests/models/llava_next/test_processor_llava_next.py +++ b/tests/models/llava_next/test_processor_llava_next.py @@ -50,19 +50,23 @@ def get_image_processor(self, **kwargs): def prepare_processor_dict(self): return {"chat_template": "dummy_template"} - # Copied from tests.models.llava.test_processor_llava.LlavaProcessorTest.test_processor_to_json_string + @unittest.skip( + "Skip because the model has no processor kwargs except for chat template and" + "chat template is saved as a separate file. Stop skipping this test when the processor" + "has new kwargs saved in config file." + ) def test_processor_to_json_string(self): - processor = self.get_processor() - obj = json.loads(processor.to_json_string()) - for key, value in self.prepare_processor_dict().items(): - # chat templates are popped from dict - if key != "chat_template": - self.assertEqual(obj[key], value) - self.assertEqual(getattr(processor, key, None), value) + pass # Copied from tests.models.llava.test_processor_llava.LlavaProcessorTest.test_chat_template_is_saved def test_chat_template_is_saved(self): processor_loaded = self.processor_class.from_pretrained(self.tmpdirname) + processor_dict_loaded = json.loads(processor_loaded.to_json_string()) + # chat templates aren't serialized to json in processors + self.assertFalse("chat_template" in processor_dict_loaded.keys()) + + # they have to be saved as separate file and loaded back from that file + # so we check if the same template is loaded processor_dict = self.prepare_processor_dict() self.assertTrue(processor_loaded.chat_template == processor_dict.get("chat_template", None)) diff --git a/tests/models/llava_onevision/test_processing_llava_onevision.py b/tests/models/llava_onevision/test_processing_llava_onevision.py index 0cfc0d46bbf8..f747c18250b6 100644 --- a/tests/models/llava_onevision/test_processing_llava_onevision.py +++ b/tests/models/llava_onevision/test_processing_llava_onevision.py @@ -60,19 +60,23 @@ def get_video_processor(self, **kwargs): def prepare_processor_dict(self): return {"chat_template": "dummy_template"} - # Copied from tests.models.llava.test_processor_llava.LlavaProcessorTest.test_processor_to_json_string + @unittest.skip( + "Skip because the model has no processor kwargs except for chat template and" + "chat template is saved as a separate file. Stop skipping this test when the processor" + "has new kwargs saved in config file." + ) def test_processor_to_json_string(self): - processor = self.get_processor() - obj = json.loads(processor.to_json_string()) - for key, value in self.prepare_processor_dict().items(): - # chat templates are popped from dict - if key != "chat_template": - self.assertEqual(obj[key], value) - self.assertEqual(getattr(processor, key, None), value) + pass # Copied from tests.models.llava.test_processor_llava.LlavaProcessorTest.test_chat_template_is_saved def test_chat_template_is_saved(self): processor_loaded = self.processor_class.from_pretrained(self.tmpdirname) + processor_dict_loaded = json.loads(processor_loaded.to_json_string()) + # chat templates aren't serialized to json in processors + self.assertFalse("chat_template" in processor_dict_loaded.keys()) + + # they have to be saved as separate file and loaded back from that file + # so we check if the same template is loaded processor_dict = self.prepare_processor_dict() self.assertTrue(processor_loaded.chat_template == processor_dict.get("chat_template", None))