transformers icon indicating copy to clipboard operation
transformers copied to clipboard

Add Aria

Open aymeric-roucher opened this issue 1 year ago • 3 comments

What does this PR do?

Add rhymes-ai/Aria to transformers!

@ArthurZucker

aymeric-roucher avatar Oct 14 '24 14:10 aymeric-roucher

@ArthurZucker I have this error that we discussed in python utils/check_modular_conversion.py:

Traceback (most recent call last):
  File "/home/ubuntu/transformers/utils/check_modular_conversion.py", line 73, in <module>
    non_matching_files += compare_files(modular_file_path, args.fix_and_overwrite)
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ubuntu/transformers/utils/check_modular_conversion.py", line 53, in compare_files
    generated_modeling_content = convert_modular_file(modular_file_path)
                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ubuntu/transformers/utils/modular_model_converter.py", line 1138, in convert_modular_file
    wrapper.visit(cst_transformers)
  File "/home/ubuntu/venv/aria/lib/python3.12/site-packages/libcst/metadata/wrapper.py", line 204, in visit
    return self.module.visit(visitor)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ubuntu/venv/aria/lib/python3.12/site-packages/libcst/_nodes/module.py", line 89, in visit
    result = super(Module, self).visit(visitor)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ubuntu/venv/aria/lib/python3.12/site-packages/libcst/_nodes/base.py", line 236, in visit
    leave_result = visitor.on_leave(self, with_updated_children)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ubuntu/venv/aria/lib/python3.12/site-packages/libcst/_visitors.py", line 71, in on_leave
    updated_node = leave_func(original_node, updated_node)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ubuntu/transformers/utils/modular_model_converter.py", line 1111, in leave_Module
    self._recursively_add_all_new_needed_functions_in_files()
  File "/home/ubuntu/transformers/utils/modular_model_converter.py", line 1097, in _recursively_add_all_new_needed_functions_in_files
    dependency, body, self.all_definitions[dependency], parent=parent
                      ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^
KeyError: 'select_best_resolution'

aymeric-roucher avatar Oct 14 '24 14:10 aymeric-roucher

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

cc @Cyrilvallez as well!

ArthurZucker avatar Oct 15 '24 11:10 ArthurZucker

I don't get this error @molbap : With from ...image_utils import PILImageResampling, I get an "unprotected import" error, even though many other models have the exact same way of importing PILImageResampling, cf image_processing_idefics3.py for instance.

aymeric-roucher avatar Nov 07 '24 20:11 aymeric-roucher

For the record: import structure should be protected in the __init__.py !

molbap avatar Nov 07 '24 20:11 molbap

Any timeline for this ? We would love to push a quantized version!

mobicham avatar Nov 21 '24 15:11 mobicham

@zucchini-nlp I still have some errors in the modular conversion (which due to modular conversion not generating files correctly)

I also have an unprotected import error that I don't find the cause for:

  • I do have some unprotected import torch in modeling_aria.py, but idefics3 has the same thing.
  • I have no torch imports in processing or image_processing files. No image imports either except for from ...utils import ImageInput, PILImageResampling but idefics3 also has the same.

Generating from input embeds seems broken, I'll investigate that. Meanwhile do you have ideas on how to solve the import issue above?

aymeric-roucher avatar Nov 25 '24 15:11 aymeric-roucher

@Cyrilvallez the modular does not export all files correctly, which makes this test fail:

Do you have an idea how to solve this? It's the last big issue before being able to merge.

aymeric-roucher avatar Nov 27 '24 14:11 aymeric-roucher

@aymeric-roucher the import error is from https://github.com/huggingface/transformers/blob/add-aria/src/transformers/models/aria/init.py file. To get all the files correctly on init from modular, you can add a __all__ = ["ObjectName1", "ObjectName"] in the modular file at the end with all the objects that need high level import.

And the __init__.py content should be

from typing import TYPE_CHECKING

from ...utils import _LazyModule
from ...utils.import_utils import define_import_structure


if TYPE_CHECKING:
    from .configuration_aria import *
    from .image_processing_aria import *
    from .modeling_aria import *
    from .processing_aria import *
else:
    import sys

    _file = globals()["__file__"]
    sys.modules[__name__] = _LazyModule(__name__, _file, define_import_structure(_file), module_spec=__spec__)

I think in that case everything should be done through modular, including processor and image processor

zucchini-nlp avatar Nov 27 '24 14:11 zucchini-nlp

@ArthurZucker the tests failing are either flaky tests failing on other models, due to modular export, or an unsolved import error that we'll investigate with @zucchini-nlp : so that's ready for final review!

aymeric-roucher avatar Nov 27 '24 14:11 aymeric-roucher

The battle continues ⚔️! Looking into the modular issue

Cyrilvallez avatar Nov 27 '24 15:11 Cyrilvallez

@aymeric-roucher, I just pushed some modifications to the modular converter and the modular itself. I think it should now behave the way you want. I did not apply the modular_converter on the new modular so you can do it yourself and check the changes it produces more easily. Let me know if it solves everything or if you still experience some issues!

Cyrilvallez avatar Nov 28 '24 18:11 Cyrilvallez

I'm unable to run the processor of this model in vLLM:

  File "/home/cyrus/miniconda3/envs/vllm/lib/python3.9/site-packages/transformers/models/aria/processing_aria.py", line 129, in __call__
    sample = sample.replace(self.tokenizer.image_token, self.tokenizer.image_token * num_crops)
  File "/home/cyrus/miniconda3/envs/vllm/lib/python3.9/site-packages/transformers/tokenization_utils_base.py", line 1108, in __getattr__
    raise AttributeError(f"{self.__class__.__name__} has no attribute {key}")
AttributeError: CachedLlamaTokenizerFast has no attribute image_token

Perhaps we need to add the image_token attribute to this processor, similar to the other VLMs?

DarkLight1337 avatar Jan 19 '25 02:01 DarkLight1337

This also happens with vanilla Transformers. Let me open a new issue on this.

Edit: Opened https://github.com/huggingface/transformers/issues/35768

DarkLight1337 avatar Jan 19 '25 02:01 DarkLight1337