TFClipModel fails to train because of None loss
Author: taymills
Pull request: no
Labels: name: bug

### System Info

transformers version: 4.21.1
Platform: MacOS BigSur 11.6.7
Python version: 3.8.13
Huggingface_hub version: 0.8.1
Tensorflow version (GPU?): 2.7.3 (False)
Flax version (CPU?/GPU?/TPU?): not installed (NA)
Jax version: not installed
JaxLib version: not installed
Using GPU in script?: no
Using distributed or parallel set-up in script?: no

@patil-suraj


### Who can help?

@patil-suraj



### Information

- [X] The official example scripts
- [ ] My own modified scripts

### Tasks

- [X] An officially supported task in the `examples` folder (such as GLUE/SQuAD, ...)
- [ ] My own task or dataset (give details below)

### Reproduction

This is the script run to attempt to fit the model to the example data. It is verbatim from the 4.21.1 docs with the addition of `model.fit`. The same error arose when working with my own project. The loss is always `None` as is `y` `y_pred` . Somewhere in the logic of https://github.com/huggingface/transformers/blob/132402d752044301b37e54405832738b16f49df6/src/transformers/modeling_tf_utils.py#L1116.

```python
from PIL import Image
import requests
from transformers import CLIPProcessor, TFCLIPModel
import tensorflow as tf

model = TFCLIPModel.from_pretrained("openai/clip-vit-base-patch32")
processor = CLIPProcessor.from_pretrained("openai/clip-vit-base-patch32")

url = "http://images.cocodataset.org/val2017/000000039769.jpg"
image = Image.open(requests.get(url, stream=True).raw)

inputs = processor(
    text=["a photo of a cat", "a photo of a dog"], images=[image, image], return_tensors="tf", padding=True
)

outputs = model(**inputs)

model.compile(optimizer=tf.keras.optimizers.Adam(learning_rate=0.00001))
model.fit(dict(inputs))
```

Results with an error of `zero gradient` because the gradients are all 0s, which I expect is caused by `y` and `y_pred` both being empty dicts.

### Expected behavior

Model.fit() on inputs from preprocessor completes a training step without error.

Comments:
Also, I do not see a test case for the fit method anywhere in https://github.com/huggingface/transformers/blob/main/tests/models/clip/test_modeling_tf_clip.py, which should probably be added

> Also, I do not see a test case for the fit method anywhere in [https://github.com/huggingface/transformers/blob/main/tests/models/clip/test_modeling_tf_clip.py](https://github.com/huggingface/transformers/blob/main/tests/models/clip/test_modeling_tf_clip.py?rgh-link-date=2022-08-17T19%3A45%3A04Z), which should probably be added

Hi! There is 
https://github.com/huggingface/transformers/blob/0ea53822f8bdc2c0c41b45bcd04fa6c031e5e700/tests/test_modeling_tf_common.py#L1406

defined in the parent class `TFModelTesterMixin`.

For the issue, could you check if there is `labels` key in `inputs`? cc @Rocketknight1 

Hi @taymills, the issue is caused by the `TFClipModel` needing `return_loss=True` to be set to return a loss - you can see this in [the model docs](https://huggingface.co/docs/transformers/model_doc/clip#transformers.TFCLIPModel).

I agree it's unintuitive that `fit()` does not set this to True by default, and I'm not sure why the issue is not detected in the tests - I'll investigate now and hopefully push a fix soon.

Update: The `test_keras_fit` test skips models that do not have a `hf_compute_loss` method. This check was added to skip the test on 'base' models like `TFBertModel` that do not have a specific output head or loss, because these models cannot be fit directly. However, `TFClipModel` does have a specific task and loss, but does not have `hf_compute_loss`.

The solution is to rewrite the check to correctly identify model types that can be fit, while still not running the test on base models that cannot. I'm working on that now.

@taymills your code sample runs correctly for me with the latest patch. Can you check and confirm it works for you too? To use the PR branch, run `pip install --upgrade git+https://github.com/huggingface/transformers.git@return_loss_fix`

Fantastic thanks for the quick response. I will give it a whirl. Also @Rocketknight1 I agree that it is non-intuitive given the docs as it is a reasonable assumption that defaulting to the "loss you probably want" implies that it actually would return said loss.

@taymills yes, that's part of this PR! When using the built-in loss, we now force `return_loss=True` for models where it is an argument. That should avoid this for CLIP and for other similar models in future.

Everything looks good! Really appreciate the quick turn around on that @Rocketknight1 . Kudos 🎉 !!!

@taymills No problem! Fixing the tests has exposed a few other issues though, which that PR will need to fix as well. Unfortunately, you're stuck in the PR branch for now, but I'll ping you and close this issue when it's merged to main!

@Rocketknight1 I am curious, when you fixed the test for the model fit for TFClipModel, did you start hitting a bunch of issues with the func `shape_list` in `tf_utils`? I am running into lots of bugs with that when trying to infer the shape.

@taymills 

My colleague is currently off. The PR is not completed yet, there are still some failing tests to fix, but I don't know if it is related to the issue you encounter.

Could you provide a short code snippet to show the issues?

Actually I have tried this with a couple of TFModels now and it appears that Transformers model fit in general does not work with tf.data.TFRecordDataset tensors. Seems it only works with EagerTensors unless I am missing something. Only way I have been able to get it to work is coercing to numpy iterator.

This is a bit off topic so feel free to ignore this as it turns out it is not germane to the current issue.

e.g.

`conftest.py`
```python
from typing import *

import io
import pickle
import random

import numpy as np
import pytest
import requests
import tensorflow as tf
import transformers
from PIL import Image
from product_ds_shared_utils.model_io.tensorflow.tfdata_helpers import (
    create_example_proto,
)

# This is an example image pulled from public dataset to use for model and data loarder smoke tests
# TODO jmills: Might be better to store this on GCS or in repo
EXAMPLE_IMAGE_URL: str = "http://images.cocodataset.org/val2017/000000039769.jpg"
# This is the schema for inputs to clip

# This is the size of the test dataset. One example image and example text is repeated this number of times
DATASET_LENGTH: int = 5
# This is the clip pretrained model name for any clip fixtures
CLIP_PRETRAINED_NAME = "openai/clip-vit-base-patch32"
# This is the clip pretrained model name for any clip fixtures
BERT_PRETRAINED_NAME = "distilbert-base-uncased"


@pytest.fixture(scope="session")
def example_image_raw() -> bytes:
    image = requests.get(EXAMPLE_IMAGE_URL, stream=True).content
    return image


@pytest.fixture(scope="session")
def example_text() -> str:
    return "This is an image of a cat"


@pytest.fixture(scope="session")
def example_dataset_clip_tfrecords(
    example_image_raw, example_text, tmpdir_factory
) -> Tuple[str, str]:
    ds_path = str(tmpdir_factory.mktemp("test_clip_model").join("dataset.tfrecords"))
    desc_path = str(tmpdir_factory.mktemp("test_clip_model").join("feature_desc.pkl"))
    example_image = Image.open(io.BytesIO(example_image_raw))
    processor = transformers.AutoProcessor.from_pretrained(CLIP_PRETRAINED_NAME)

    feature_dict_processed = dict(
        processor(
            images=example_image, text=example_text, return_tensors="tf", padding=True
        )
    )

    with tf.io.TFRecordWriter(ds_path) as file_writer:
        for i in range(0, DATASET_LENGTH, 1):
            feature_dict_processed["labels"] = tf.constant([random.choice([0, 1])])
            example_proto, feature_description = create_example_proto(
                feature_dict_processed
            )
            file_writer.write(example_proto.SerializeToString())
    with open(desc_path, "wb") as f:
        pickle.dump(feature_description, f)

    return ds_path, desc_path
```

test_train.py - note `trainer.model` is a TFClipModel.from_pretrained
```python
"""
Test trainer class using clip pretrained model
"""
from typing import *

import pathlib
import pickle

import pytest
import tensorflow as tf
import tensorflow_addons as tfa

from product_embeddings.trainer import EmbeddingTrainer

MODEL_NAME: str = "openai/clip-vit-base-patch32"


@pytest.fixture(scope="function")
def parsed_dataset_tfrecords(example_dataset_clip_tfrecords):
    dataset_path, schema_path = example_dataset_clip_tfrecords
    with open(schema_path, "rb") as f:
        schema = pickle.load(f)

    def decode_fn(record_bytes: bytes) -> Tuple[Dict[str, tf.Tensor], tf.Tensor]:
        parsed_example = tf.io.parse_single_example(record_bytes, schema)
        parsed_example["input_ids"] = tf.io.parse_tensor(
            parsed_example["input_ids"], tf.int32
        )
        parsed_example["pixel_values"] = tf.io.parse_tensor(
            parsed_example["pixel_values"], tf.float32
        )
        parsed_example["attention_mask"] = tf.io.parse_tensor(
            parsed_example["attention_mask"], tf.int32
        )
        parsed_example["labels"] = tf.io.parse_tensor(
            parsed_example["labels"], tf.int32
        )
        labels = parsed_example.pop("labels")
        return parsed_example, labels

    dataset = tf.data.TFRecordDataset([str(dataset_path)]).map(
        decode_fn, num_parallel_calls=tf.data.AUTOTUNE
    )

    return dataset


def test_dataset_format(parsed_dataset_tfrecords):
    for features, label in parsed_dataset_tfrecords:
        assert list(features.keys()) == ["attention_mask", "input_ids", "pixel_values"]
        for key, val in features.items():
            assert isinstance(
                val, tf.Tensor
            ), f"Feature {key} is not a tensor but got {val}"
        assert isinstance(label, tf.Tensor)
        assert features["input_ids"].shape == (1, 9)
        assert features["pixel_values"].shape == (1, 3, 224, 224)
        assert label.shape == (1)


def test_clip_model_trainer_tfrecords(tmpdir_factory, parsed_dataset_tfrecords):
    checkpoint_path = pathlib.Path(
        tmpdir_factory.mktemp("test_clip_model").join("checkpoints")
    )
    log_path = pathlib.Path(tmpdir_factory.mktemp("test_clip_model").join("logs"))

    trainer = EmbeddingTrainer(
        model_name=MODEL_NAME,
        checkpoint_dir=str(checkpoint_path),
        log_dir=str(log_path),
    )
    trainer.compile(loss=tfa.losses.contrastive_loss)

    trainer.model.run_train(
        training_dataset=parsed_dataset_tfrecords.as_numpy_iterator(),
        validation_dataset=parsed_dataset_tfrecords.as_numpy_iterator(),
        epochs=1,
    )
```

@taymills That's quite an odd issue - it's definitely unrelated to this one, but if you haven't managed to figure it out, can you copy it into a new issue and tag me? It looks like your model is coming from some external library in the second example though (our models don't have a `run_train` method), so if that's the case we probably can't do much about the problem.