huggingface / transformers

🤗 Transformers: State-of-the-art Machine Learning for Pytorch, TensorFlow, and JAX.
https://huggingface.co/transformers
Apache License 2.0
134.03k stars 26.8k forks source link

Add Deformable DETR to the object detection pipeline tests #19024

Closed NielsRogge closed 1 year ago

NielsRogge commented 2 years ago

Deformable DETR was added in #17281, however I had to disable the model for the object detection pipeline tests, as it fails.

However, the model runs just fine with the pipeline as shown in this notebook.

cc @Narsil

Also cc'ing @mishig25, it may be beneficial to add a "threshold" button to the object detection widget, as Deformable DETR for instance only detects objects on the cats image with a threshold of 0.7, whereas the current threshold is set to 0.9.

github-actions[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

NielsRogge commented 2 years ago

cc @Narsil, could you take a look here please?

LysandreJik commented 2 years ago

@NielsRogge, have you taken a look at the pipeline's error when adding Deformable DETR? Usually the errors are clearly shown. @Narsil isn't responsible for adding the tests for the model you've contributed to the pipeline but I'm sure he'd be happy to help if you have a specific error you can't solve, but please post the error message here to make it easier. Thanks!

Narsil commented 2 years ago

Have you tried checking the failing test ?

tests/pipelines/test_pipelines_common.py:256: in run_batch_test
    for item in pipeline(data(10), batch_size=4):
src/transformers/pipelines/pt_utils.py:111: in __next__
    item = next(self.iterator)
src/transformers/pipelines/pt_utils.py:108: in __next__
    return self.loader_batch_item()

The test fails because it DOES fail, and that something is wrong with Detr. If you don't feel comfortable doing the fix it's fine.

But I'll ask you what we ask of all the community.

Share what you have tried, show the error logs and explain better your issue.

"It works in my colab" is not enough. I don't have time to check your colab, I need a small reproducible script (without colab). And "it works on my computer" is not good enough when the tests fail.

The test actually do showcase really well what's wrong. You do have to read the stacktrace though.

I am very willing to help anyone, but you never ever show either gratitude nor even show attempts at even trying.

NielsRogge commented 2 years ago

I am very willing to help anyone, but you never ever show either gratitude nor even show attempts at even trying.

Sincere apology to not include the error. I'll add the error trace and take a look myself.

NielsRogge commented 2 years ago

So the test that fails is the following (I ran RUN_SLOW=yes tests/pipelines/test_pipelines_object_detection.py by enabling what was disabled as explained in the original post above):

tests/pipelines/test_pipelines_object_detection.py::ObjectDetectionPipelineTests::test_pt_DeformableDetrConfig_DeformableDetrForObjectDetection_notokenizer_DeformableDetrFeatureExtractor

It errors with:

        def run_batch_test(pipeline, examples):
            # Need to copy because `Conversation` are stateful
            if pipeline.tokenizer is not None and pipeline.tokenizer.pad_token_id is None:
                return  # No batching for this and it's OK

            # 10 examples with batch size 4 means there needs to be a unfinished batch
            # which is important for the unbatcher
            def data(n):
                for _ in range(n):
                    # Need to copy because Conversation object is mutated
                    yield copy.deepcopy(random.choice(examples))

            out = []
            for item in pipeline(data(10), batch_size=4):
                out.append(item)
            self.assertEqual(len(out), 10)

>       run_batch_test(pipeline, examples)

tests/pipelines/test_pipelines_common.py:260: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/pipelines/test_pipelines_common.py:256: in run_batch_test
    for item in pipeline(data(10), batch_size=4):
src/transformers/pipelines/pt_utils.py:111: in __next__
    item = next(self.iterator)
src/transformers/pipelines/pt_utils.py:108: in __next__
    return self.loader_batch_item()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <transformers.pipelines.pt_utils.PipelineIterator object at 0x7feffeffea60>

    def loader_batch_item(self):
        """
        Return item located at `loader_batch_index` within the current `loader_batch_data`.
        """
        if isinstance(self._loader_batch_data, torch.Tensor):
            # Batch data is simple tensor, just fetch the slice
            result = self._loader_batch_data[self._loader_batch_index]
        else:
            # Batch data is assumed to be BaseModelOutput (or dict)
            loader_batched = {}
            for k, element in self._loader_batch_data.items():
                if k in {"hidden_states", "past_key_values", "attentions"} and isinstance(element, tuple):
                    # Those are stored as lists of tensors so need specific unbatching.
                    if isinstance(element[0], torch.Tensor):
                        loader_batched[k] = tuple(el[self._loader_batch_index].unsqueeze(0) for el in element)
                    elif isinstance(element[0], np.ndarray):
                        loader_batched[k] = tuple(np.expand_dims(el[self._loader_batch_index], 0) for el in element)
                    continue
>               if isinstance(element[self._loader_batch_index], torch.Tensor):
E               IndexError: index 2 is out of bounds for dimension 0 with size 2

I'll definitely need your help here @LysandreJik @Narsil as I am not really sure what is going on here. Sorry again for not including the original error and I appreciate any help.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

NielsRogge commented 1 year ago

This was fixed in #19678.