Skip to content

Conversation

@ydshieh
Copy link
Collaborator

@ydshieh ydshieh commented Sep 20, 2023

What does this PR do?

Update tiny model information and pipeline tests

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Sep 20, 2023

The documentation is not available anymore as the PR was closed or merged.

@ydshieh
Copy link
Collaborator Author

ydshieh commented Sep 20, 2023

@sanchit-gandhi

The tests/models/vits/test_modeling_vits.py::VitsModelTest::test_pipeline_text_to_audio fails (in this PR) with TypeError: forward() got an unexpected keyword argument 'num_return_sequences'. You can see it here or the artifact tab.

I am not sure if ViTsModel should be in MODEL_FOR_TEXT_TO_WAVEFORM_MAPPING_NAMES (and therefore used by TextToAudioPipelineTests where generate is used.

Could you check this to see if a fix is required?

You can checkout this branch update_tiny (if you use HF repo. locally) and run it

python3 -m pytest -v tests/models/vits/test_modeling_vits.py::VitsModelTest::test_pipeline_text_to_audio

The full error log

self = <tests.models.vits.test_modeling_vits.VitsModelTest testMethod=test_pipeline_text_to_audio>

    @is_pipeline_test
    @require_torch
    def test_pipeline_text_to_audio(self):
>       self.run_task_tests(task="text-to-audio")

tests/test_pipeline_mixin.py:413: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
tests/test_pipeline_mixin.py:171: in run_task_tests
    self.run_model_pipeline_tests(
tests/test_pipeline_mixin.py:209: in run_model_pipeline_tests
    self.run_pipeline_test(task, repo_name, model_architecture, tokenizer_name, processor_name, commit)
tests/test_pipeline_mixin.py:298: in run_pipeline_test
    task_test.run_pipeline_test(pipeline, examples)
tests/pipelines/test_pipelines_text_to_audio.py:185: in run_pipeline_test
    outputs = speech_generator(["This is great !", "Something else"], forward_params=forward_params)
../.pyenv/versions/3.8.12/lib/python3.8/site-packages/transformers/pipelines/text_to_audio.py:138: in __call__
    return super().__call__(text_inputs, **forward_params)
../.pyenv/versions/3.8.12/lib/python3.8/site-packages/transformers/pipelines/base.py:1121: in __call__
    outputs = list(final_iterator)
../.pyenv/versions/3.8.12/lib/python3.8/site-packages/transformers/pipelines/pt_utils.py:124: in __next__
    item = next(self.iterator)
../.pyenv/versions/3.8.12/lib/python3.8/site-packages/transformers/pipelines/pt_utils.py:125: in __next__
    processed = self.infer(item, **self.params)
../.pyenv/versions/3.8.12/lib/python3.8/site-packages/transformers/pipelines/base.py:1046: in forward
    model_outputs = self._forward(model_inputs, **forward_params)
../.pyenv/versions/3.8.12/lib/python3.8/site-packages/transformers/pipelines/text_to_audio.py:114: in _forward
    output = self.model(**model_inputs, **kwargs)[0]
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = VitsModel(
  (text_encoder): VitsTextEncoder(
    (embed_tokens): Embedding(38, 16)
    (encoder): VitsEncoder(
      ...    (dropout): Dropout(p=0.0, inplace=False)
    )
    (conv_proj): Conv1d(16, 32, kernel_size=(1,), stride=(1,))
  )
)
args = ()
kwargs = {'attention_mask': tensor([[1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
         1, 1, 1]]...  8,  0, 19,  0, 18,  0,  8,  0, 19,  0, 37,
          0, 25,  0,  7,  0, 26,  0, 33,  0]]), 'num_return_sequences': 2}
forward_call = <bound method VitsModel.forward of VitsModel(
  (text_encoder): VitsTextEncoder(
    (embed_tokens): Embedding(38, 16)...   (dropout): Dropout(p=0.0, inplace=False)
    )
    (conv_proj): Conv1d(16, 32, kernel_size=(1,), stride=(1,))
  )
)>

    def _call_impl(self, *args, **kwargs):
        forward_call = (self._slow_forward if torch._C._get_tracing_state() else self.forward)
        # If we don't have any hooks, we want to skip the rest of the logic in
        # this function, and just call forward.
        if not (self._backward_hooks or self._backward_pre_hooks or self._forward_hooks or self._forward_pre_hooks
                or _global_backward_pre_hooks or _global_backward_hooks
                or _global_forward_hooks or _global_forward_pre_hooks):
>           return forward_call(*args, **kwargs)
E           TypeError: forward() got an unexpected keyword argument 'num_return_sequences'

../.pyenv/versions/3.8.12/lib/python3.8/site-packages/torch/nn/modules/module.py:1501: TypeError

@sanchit-gandhi
Copy link
Contributor

Hey @ydshieh! The VITS model is registered under the correct mapping, and the pipeline class is correct in that it only calls .generate if the model is an auto-regressive model:

if self.model.can_generate():
output = self.model.generate(**model_inputs, **kwargs)
else:
output = self.model(**model_inputs, **kwargs)[0]

The problem is in the testing code, namely that num_return_sequences and do_sample are always passed to the forward_params of the pipeline:

forward_params = {"num_return_sequences": 2, "do_sample": True}

We can set the forward_params in the test based on whether the model can generate:

forward_params = {"num_return_sequences": 2, "do_sample": True} if speech_generator.model.can_generate() else {}

cc @ylacombe as well for info

@ydshieh
Copy link
Collaborator Author

ydshieh commented Sep 21, 2023

@sanchit-gandhi Thank you for the information!

Comment on lines +184 to +186
forward_params = (
{"num_return_sequences": 2, "do_sample": True} if speech_generator.model.can_generate() else {}
)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

def is_pipeline_test_to_skip(
self, pipeline_test_casse_name, config_class, model_architecture, tokenizer_name, processor_name
):
return True
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably better not to add entries into pipeline_model_mapping here - by excluding this model in the corresponding pipeline (test) classes.

@ydshieh ydshieh marked this pull request as ready for review September 22, 2023 08:23
TF_MODEL_FOR_DOCUMENT_QUESTION_ANSWERING_MAPPING_NAMES = OrderedDict(
[
("layoutlm", "TFLayoutLMForQuestionAnswering"),
("layoutlmv3", "TFLayoutLMv3ForQuestionAnswering"),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The torch part has this model in MODEL_FOR_DOCUMENT_QUESTION_ANSWERING_MAPPING_NAMES

@ydshieh ydshieh requested a review from LysandreJik September 25, 2023 07:42
Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work @ydshieh


# Need this class in oder to create tiny model for `bark`
# TODO (@Yoach) Implement actual test methods
@unittest.skip("So far all tests will fail.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be removed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original motivation to have this BarkModelTest is so we can create the tiny model for bark. However, @sanchit-gandhi mentioned

Bark is irregular in the sense that it's a concatenation of three auto-regressive models, meaning there's no notion of a forward pass

see here, and this BarkModelTest doesn't make sense.

I will remove this test class.

@ydshieh ydshieh merged commit d9e4bc2 into main Sep 25, 2023
@ydshieh ydshieh deleted the update_tiny branch September 25, 2023 16:08
Copy link
Contributor

@sanchit-gandhi sanchit-gandhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Audio related changes LGTM - thanks @ydshieh!

parambharat pushed a commit to parambharat/transformers that referenced this pull request Sep 26, 2023
* Update tiny model summary file

* add to pipeline tests

* revert

* fix import

* fix import

* fix

* fix

* update

* update

* update

* fix

* remove BarkModelTest

* fix

---------

Co-authored-by: ydshieh <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants