Add TF swiftformer
Author: joaocmd
Pull request: yes

# What does this PR do?

Adds the TensorFlow version of the "SwiftFormer".

Fixes #22771 

## Before submitting
- [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [X] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md#start-contributing-pull-requests),
      Pull Request section?
- [X] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Please add a link
      to it if that's the case. https://github.com/huggingface/transformers/issues/22771
- [x] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/main/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/main/docs#writing-source-documentation).
- [x] Did you write any new necessary tests?


## Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@amyeroberts @D-Roberts

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_23342). All of your documentation changes will be reflected on that endpoint.

Hi @joaocmd, 
Rapid work on opening the TF port! Let me or @Rocketknight1 know when the PR is ready for review or you experience any issues when porting. 

Hi @Rocketknight1, could I get some pointers as to why I get errors like in most of the tests:

```
E               ValueError: Exception encountered when calling layer 'tf_swift_former_model_18' (type TFSwiftFormerModel).
E               
E               The following keyword arguments are not supported by this model: ['input_ids'].
E               
E               Call arguments received by layer 'tf_swift_former_model_18' (type TFSwiftFormerModel):
E                 • pixel_values={'pixel_values': 'tf.Tensor(shape=(13, 224, 224, 3), dtype=float32)'}
E                 • output_hidden_states=None
E                 • return_dict=None
E                 • training=False

src/transformers/modeling_tf_utils.py:500: ValueError
```

The PyTorch model has this following docstring but I don't see where the input_ids part is being taken care of.
```py
"""
    Here we also overwrite some of the tests of test_modeling_common.py, as SwiftFormer does not use input_ids, inputs_embeds,
    attention_mask and seq_length.
"""
```

Thanks!

It seems like it is entering in the `else` statement at line 581 of `src/transformers/modeling_tf_utils.py`:

```python
if "args" in output:
    if output["args"] is not None and is_tf_symbolic_tensor(output["args"]):
        tensor_name = output["args"].name.split(":")[0]
        output[tensor_name] = output["args"]
    else:
        # `args` in this case is always the first parameter, then `input_ids`
        output["input_ids"] = output["args"]

    del output["args"]
```

Thus it is injecting the `input_ids` argument into the dictionary.

@amyeroberts @Rocketknight1 How should I get around this? It must be some misconfiguration in my tests or models.



@joaocmd Just looking at the error and the CI runs, I think the issue might be a missing `@unpack_inputs` decorator on the call method for the MainLayer class

> @joaocmd Just looking at the error and the CI runs, I think the issue might be a missing `@unpack_inputs` decorator on the call method for the MainLayer class

Thank you @amyeroberts! It seems like that wasn't causing any issue (yet), but thanks to your comment I found out that I had a duplicate `@unpack_inputs` in one of the models.

Hi @amyeroberts and @Rocketknight1, can I get some help with the tests that are still failing? I'm getting `ValueError: cannot reshape array of size 10368 into shape (3,3,3,24)` for these two tests:
* `tests/models/swiftformer/test_modeling_tf_swiftformer.py::TFSwiftFormerModelTest::test_compile_tf_model`
* `tests/models/swiftformer/test_modeling_tf_swiftformer.py::TFSwiftFormerModelTest::test_save_load`

But I don't understand exactly what is being reshaped into the wrong shape. Could I get some insight as to what these tests are doing and why it might be failing? Thanks!

Hi @joaocmd, there's been some large updates to our TF models regarding how they're built - @Rocketknight1 can give you more details :) 

Are these errors happening if you rebase on `main`? 

> Hi @joaocmd, there's been some large updates to our TF models regarding how they're built - @Rocketknight1 can give you more details :)
> 
> Are these errors happening if you rebase on `main`?

Hi @amyeroberts, just rebased the branch. I think it's failing on the same tests but the error on these two tests changed to:
```
NotImplementedError: Could not infer input image shape from config, please override input_signature to specify input shapes.
```

Looking at the stack trace it seems like the image size should have been specified:
```python
if hasattr(vision_config, "image_size"):
    pixel_values_shape[2] = pixel_values_shape[3] = vision_config.image_size
elif hasattr(vision_config, "input_size"):
    pixel_values_shape[2] = pixel_values_shape[3] = vision_config.input_size
else:
   raise NotImplementedError( # <------ this error here
        "Could not infer input image shape from config, please override input_signature to specify input shapes."
    )
```

Shouldn't this also affect the original model?


@joaocmd Regarding the error, no, it shouldn't affect the original model. `image_size` is a parameter we add in the configs, even if it's not always used by the model as it's often important for parameterizing other things or understanding. We allow [this here](https://github.com/huggingface/transformers/blob/468aed39afffafe417819a309a4e6d45d2a9e8f4/utils/check_config_attributes.py#L187). It should have been added, and we can add in this PR, but the PT model can do without. 

You'll notice that the error is being raise in `modeling_tf_utils.py`. This is because when constructing a TF model, we have to pass in dummy inputs to build it. In PyTorch this isn't necessary, because we explicitly set the input and output dimensions when creating each layer, so the weight matrices can be created immediately. `image_size` is needed to know the shape of the inputs to pass in. 

As a side note, did you force push after rebasing? From the PR history, it looks like you might not have. As rebasing is a form of "rewriting history" it's necessary to force push.








Thanks @amyeroberts, understood. As for the rebase, I had not done one in quite some time and it seems like I did mess it up. I think that is now fixed.

Since I started this PR I have had a fundamental question about huggingface's approach to tensorflow models. The default in TensorFlow is NHWC while in PyTorch it is NCHW, how should I approach this difference in my PR? Based on `modeling_tf_vit.py` I suppose the correct approach is to assume that images are given in PyTorch format and transpose them in the first block, is that so? How does that affect the following blocks?
Also, if we were implementing a model for semantic segmentation, which would return an image with the same size as the original one, would that be returned in the PyTorch format or the default TensorFlow format?

Thank you!

@joaocmd The pattern we use for the TF vision models is to transpose the NCHW format in the first MainLayer class e.g. [here](https://github.com/huggingface/transformers/blob/868363abb9e72a638b4710d1f5ef1199839b3eec/src/transformers/models/resnet/modeling_tf_resnet.py#L337C18-L337C18) and then transpose back, if pixel values are returned e.g. [here](https://github.com/huggingface/transformers/blob/868363abb9e72a638b4710d1f5ef1199839b3eec/src/transformers/models/resnet/modeling_tf_resnet.py#L348). For some of the older models e.g. ViT this pattern may not have been applied, as these were the first models to be added. 

This pattern means the model is written in the TF compatible NHWC format throughout, but all of our vision models accept and return images in NCHW. 

Thank you @amyeroberts, that makes sense. I've already updated it to match the pattern.

I'm still having some trouble with the `test_compile_tf_model`. Initially it was failing because it was passing a shape `(None, 56, 56, 48)` to a `reshape` (https://github.com/huggingface/transformers/pull/23342/commits/204e216e6047d83775cfb5f0d928b378b73d2e84#diff-7f093399e807b53ca4b63460f610dcc550c2937cb18cd513d71dc49ce6e1b699R385).
I changed the line to use `[-1, width * height, channels]` as shape, which seems like it fixed that case. However, now it is failing because a shape `(None, None, None, 48)` is being passed to that reshape call. Is this expected of this test? According to the stack trace it seems like it's being triggered by a `tf.keras.Model.save()` (https://github.com/joaocmd/transformers/blob/add_tf_swiftformer/tests/test_modeling_tf_common.py#L711).

I've also noticed that there was an overhaul to the serving and dummy_inputs interface (https://github.com/huggingface/transformers/commit/814de8fac7456bd2ce50d1847505da829761bfdc). But maybe @Rocketknight1 can better explain the consequences of this change to mine (and other) PRs.

@joaocmd Yes, there was a big refactor of the `serving_output` logic. For most models, there's no need to have `serving_output`, `dummy_inputs` or `serving` implemented. You should be able to remove these and have the `test_prepare_serving_output` test pass. 

Looking at the CI run, I don't see `test_compile_tf_model` failing. Were you able to resolve? Or perhaps are you refering to `test_save_load`?

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](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md) are likely to be ignored.

Hi @amyeroberts! Sorry for the late response as I've been quite busy... It was failing more tests on my local machine than on the CI run, but after merging the main branch locally they now seem to match.
I am currently struggling with `test_save_load`:
```
ValueError: cannot reshape array of size 10368 into shape (3,3,3,24)
```

I can't find the reason for this error. I set up a breakpoint and found that the `symbolic_weight_name` at that point is `kernel:0`, so I assume it belongs to some convolutional layer, but I didn't get any further than that. Do you have any suggestions? Thank you!

Edit:

I believe the weight belongs to the patch embeddings layer, which I initialized with a `tf.keras.Sequential` call:
```python
self.patch_embedding = tf.keras.Sequential(
    [
        tf.keras.layers.ZeroPadding2D(padding=(1, 1)),
        tf.keras.layers.Conv2D(out_chs // 2, kernel_size=3, strides=2),
        tf.keras.layers.BatchNormalization(
            epsilon=config.batch_norm_eps, momentum=0.9
        ),  # FIXME: is this the equivalent momentum?
        tf.keras.layers.Activation("relu"),
        tf.keras.layers.ZeroPadding2D(padding=(1, 1)),
        tf.keras.layers.Conv2D(out_chs, kernel_size=3, strides=2),
        tf.keras.layers.BatchNormalization(
            epsilon=config.batch_norm_eps, momentum=0.9
        ),  # FIXME: is this the equivalent momentum?
        tf.keras.layers.Activation("relu"),
    ],
    name="patch_embeddings",
)
```

I think the problem is that both `Conv2D` are being given the same name, what is the correct approach for this? Should I rewrite the pytorch version to not use `nn.Sequential`?

@joaocmd I would suggest rewriting the torch version to not use sequential, but only for the purposes of debugging i.e. we wouldn't commit these changes to main. This way you'll be able to go line by line comparing the TF and PT outputs and seeing where any shape differences are coming from. 

Hi @amyeroberts I might be misunderstanding something but I think `test_save_load` does not test any PyTorch to TensorFlow equivalence. I think the problem is that when the two convolutional layers inside the `Sequential` module are saved they are stored under the same name, so a shape mismatch happens. Do I understand this correctly?

@joaocmd Ah, apologies, I misread your comment. Yes, I believe you're right about the the naming issue. What I suggest is follow the pattern in other ports where `nn.Sequential` has been used. For example in deit, for the [sequential block in PT](https://github.com/huggingface/transformers/blob/3f9cb335047315edfd4b6ad666ef148e98cc4850/src/transformers/models/deit/modeling_deit.py#L587), a [new layer is implemented for TF](https://github.com/huggingface/transformers/blob/3f9cb335047315edfd4b6ad666ef148e98cc4850/src/transformers/models/deit/modeling_tf_deit.py#L698).

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](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md) are likely to be ignored.

@joaocmd From next week I'll be off for a few weeks. If you have any implementation questions, please ask @Rocketknight1 :)  

Hi @Rocketknight1, could you give me some pointers on the three remaining tests? I haven't looked closely into `test_modeling_tf_swiftformer.py::TFSwiftFormerModelIntegrationTest::test_inference_image_classification_head` yet because I think it makes sense to leave that one for last, but correct me if I'm wrong.

However, I am trying to understand what is wrong with `TFSwiftFormerModelTest::test_save_load - AssertionError: 0.42552373 not less than or equal to 1e-05` but I have come to no conclusion yet.

There is also this current error that might be due to some misnamed layers, but I am not sure: `tests/models/swiftformer/test_modeling_tf_swiftformer.py::TFSwiftFormerModelTest::test_pt_tf_model_equivalence - AttributeError: patch_embed.patch_embeddings.0.weight not found in PyTorch model`.

Thank you!

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](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md) are likely to be ignored.

@joaocmd Are you still working on this? If so, @Rocketknight1 could you help?

Hi @amyeroberts , I haven't made any changes since my last comment as I was stuck and had some other responsibilities. I would like to finish the issue especially because I believe it's very close to finishing.

Hi, I'm sorry, I'm not sure how I missed your last comment - this is entirely my fault! Let me investigate the errors you were getting and I'll see if we can get this PR over the line.

Hi @joaocmd, I just took a look now. The cause of the errors is that weight names are not being matched up correctly between the saved checkpoint and the model. The reason for that is that, annoyingly, you need to pass `name` args to all the layers you create.

For example, in this block:

```python
for i in range(len(layer_depths)):
    stage = TFSwiftFormerStage(config, index=i)
    self.network.append(stage)
    if i >= len(layer_depths) - 1:
        break
    if downsamples[i] or embed_dims[i] != embed_dims[i + 1]:
        # downsampling between two stages
        self.network.append(TFSwiftFormerEmbeddings(config, index=i))
```
Both `TFSwiftFormerStage` and `TFSwiftFormerEmbeddings` will need a `name` argument here. When setting the `name` args, we generally try to choose them so they match up with the PyTorch state dict, so that weights can be cross-loaded between frameworks. This means that in this case, the names would need to be `network_._0`, `network_._1` and so on. The index should be the position of the layer in the `self.network` list, so that `self.network[i].name == f"network_._{i}"`.

Hopefully after all the layers get name arguments correctly, weight loading and cross-loading should just start working! There'll probably be other issues after that, but we should be able to work through them, so feel free to ping me here if you can't figure them out - the core of the port looks quite good!

Hi @Rocketknight1, thanks a lot for your input! I printed out the transformed names of the pytorch and tensorflow weights and have a few questions.

First, I didn't find anything regarding the `{name}_._{index}` annotation. When looking at the `state_dict` I find names like `encoder.network.0.blocks.0`, but where did the underscores go?

As for the batch norm layers like in the `TFSwiftFormerPatchEmbeddingsSequential`:
```python
        self.zero_padding = tf.keras.layers.ZeroPadding2D(padding=(1, 1))
        self.conv1 = tf.keras.layers.Conv2D(out_chs // 2, kernel_size=3, strides=2, name="0")
        self.batch_norm1 = tf.keras.layers.BatchNormalization(
            epsilon=config.batch_norm_eps, momentum=0.9, name="1"
        ) # FIXME: is this the equivalent momentum?
        self.conv2 = tf.keras.layers.Conv2D(out_chs, kernel_size=3, strides=2, name="3")
        self.batch_norm2 = tf.keras.layers.BatchNormalization(
            epsilon=config.batch_norm_eps, momentum=0.9, name="4"
        ) # FIXME: is the correct momentum
   ```
   
   Looking at the pytorch state dict seems like we get:
   ```json
  "patch_embed.patch_embedding.0.weight": "patch_embed.patch_embedding.0.weight",
  "patch_embed.patch_embedding.0.bias": "patch_embed.patch_embedding.0.bias",
  "patch_embed.patch_embedding.1.weight": "patch_embed.patch_embedding.1.weight",
  "patch_embed.patch_embedding.1.bias": "patch_embed.patch_embedding.1.bias",
  "patch_embed.patch_embedding.1.moving_mean": "patch_embed.patch_embedding.1.running_mean",
  "patch_embed.patch_embedding.1.moving_variance": "patch_embed.patch_embedding.1.running_var",
  ```
  
Whereas in the tensorflow version I only found:
```json
  "patch_embed.patch_embedding.0.weight",
  "patch_embed.patch_embedding.0.bias",
  "patch_embed.patch_embedding.1.weight",
  "patch_embed.patch_embedding.1.bias",
```

Are those being associated somewhere else? Especially referring to the moving means and variances.

Thank you!

I'm now getting `tests/models/swiftformer/test_modeling_tf_swiftformer.py::TFSwiftFormerModelTest::test_pt_tf_model_equivalence - AssertionError: Tuples differ: (13, 7, 7, 220) != (13, 220, 7, 7)`.

@amyeroberts you mentioned this last time I asked you regarding model inputs/outputs:

> This pattern means the model is written in the TF compatible NHWC format throughout, but all of our vision models accept and return images in NCHW.

My question is where should I do the transposition? Should I do a for loop after running the encoder and transpose each one of the hidden states? Thank you!

@joaocmd Transposing the input (NCHW) to the TF compatible mode (NHWC) should be done in the `ModelNameMainLayer` class e.g. like here for [resnet](https://github.com/huggingface/transformers/blob/e9dbd3926317a4effb1d033d8454ff18280d0b7d/src/transformers/models/resnet/modeling_tf_resnet.py#L337) and then permuted back to NCHW [format before returning](https://github.com/huggingface/transformers/blob/e9dbd3926317a4effb1d033d8454ff18280d0b7d/src/transformers/models/resnet/modeling_tf_resnet.py#L348)

Hi @joaocmd, there's a few points here!

Firstly, regarding the underscores, we use them in TF names for various reasons. Our weight loading will correctly substitute `_._` for the `.` in a PyTorch state dict name, so the weights should match if you use them! Issues like this substitution, and the changes of name from e.g. `moving_variance` to `running_var`, are handled [here](https://github.com/huggingface/transformers/blob/main/src/transformers/modeling_tf_pytorch_utils.py#L241).

Also, the second issue you're encountering is because PyTorch convolutions are by default `channels_first` (NCHW), but TF convolutions are by default `channels_last` (NHWC). As a result, what probably happened in that test is that one of the spatial dimensions was incorrectly treated as the channels dimension.

Annoyingly, however, it doesn't work to just replace TF convolutions with `channels_first`, because TF does not correctly support channels-first convolutions on CPU. Instead, we recommend that you keep TF convolutions as `channels_last`, but instead transpose inputs before and afterwards. This is [actually the way most GPUs implement channels-first convolutions](https://docs.nvidia.com/deeplearning/performance/dl-performance-convolutional/index.html#tensor-layout) anyway!

If there is a series of convolutions, it's okay to transpose once in the beginning to NHWC and then again at the end to NCHW. However, we recommend getting back to NCHW as fast as possible once you're not doing convolutions anymore, or else the dimensions in the rest of the code will not match the PyTorch original!

Hi @Rocketknight1, could I get some pointers as to fix the remaining documentation/styling issues?

@joaocmd We recently updated our library formatting to use ruff. To resolve you'll need to: 
* rebase on main
* Uninstall black: `pip uninstall black`
* Install necessary formatting libraries: `pip install .[quality]`
* Format the code: `make fixup`
* Push any of the changes applied. You'll need to force push after rebasing. 

> @joaocmd We recently updated our library formatting to use ruff. To resolve you'll need to:
> 
>     * rebase on main
> 
>     * Uninstall black: `pip uninstall black`
> 
>     * Install necessary formatting libraries: `pip install .[quality]`
> 
>     * Format the code: `make fixup`
> 
>     * Push any of the changes applied. You'll need to force push after rebasing.

Hi @amyeroberts, I am getting this error when I run the command:
```
> make fixup                 
make: execvp: /bin/sh: Argument list too long
make: *** [Makefile:10: modified_only_fixup] Error 127
```


Hi @joaocmd, if you get that error you can try running just `make style`, which is the main command that `make fixup` runs. However, it looks like your fork is a little behind `transformers:main` - try doing the following before you run `make fixup` or `make style`!

- "Sync upstream" on your fork in the Github browser interface
- In your local repo, change to `main`, pull the latest version, then change back to `add_tf_swiftformer`
- Rebase `add_tf_swiftformer` onto `main`
- Force push (`push -f`) the `add_tf_swiftformer` branch
- Now `pip uninstall black` and `pip install .[quality]` once again to make sure you're getting the latest style dependencies

Thanks @Rocketknight1. I had already rebased based on @amyeroberts' comment. These are the commands I followed and everything seemed alright:

```
git fetch upstream/main
git rebase upstream/main
```

Running `make style` does not seem to change anything:

```zsh
(venv) ➜  transformers git:(add_tf_swiftformer) ✗ make quality
ruff check examples tests src utils setup.py conftest.py
ruff format --check examples tests src utils setup.py conftest.py
2691 files left unchanged
python utils/custom_init_isort.py --check_only
python utils/sort_auto_mappings.py --check_only
python utils/check_doc_toc.py
```

Hi @joaocmd, yep, it looks like everything's been done properly. I checked the tests and it looks like it's just a few small issues:

1) The [repository consistency test](https://app.circleci.com/pipelines/github/huggingface/transformers/79103/workflows/fc012c2a-b047-4538-8f42-355d168ad5c2/jobs/1009858) is complaining that some docstrings don't match the object's signature:
```
The following objects docstrings do not match their signature. Run `make fix-copies` to fix this.
- TFSwiftFormerForImageClassification
- TFSwiftFormerModelThe following objects docstrings contain templates you need to fix: search for `<fill_type>` or `<fill_docstring>`.
- SwiftFormerConfig
```

2) [Docstring tests](https://app.circleci.com/pipelines/github/huggingface/transformers/79103/workflows/2fefbfe6-126b-40fd-8a68-a0d373a22574/jobs/1009866) are failing because the checkpoint `MBZUAI/swiftformer-xs` does not have TF weights. The CI executes all the doctests in the file, so we'll get an error if that doctest doesn't run. Adding safetensors or TF weights to that checkpoint would fix this!

3) There are some errors involving the output loss shape. We expect models to either return a loss per sample with shape `(BATCH_SIZE,)` or a single loss for the batch with shape `(1,)`. You may get an error if you return a scalar loss with null shape - try checking the shapes of the losses for the classification model!

> [Docstring tests](https://app.circleci.com/pipelines/github/huggingface/transformers/79103/workflows/2fefbfe6-126b-40fd-8a68-a0d373a22574/jobs/1009866) are failing because the checkpoint MBZUAI/swiftformer-xs does not have TF weights. The CI executes all the doctests in the file, so we'll get an error if that doctest doesn't run. Adding safetensors or TF weights to that checkpoint would fix this!

Hi @Rocketknight1, thanks for the comment. Is there any place where I can read on how to do do that?

> The [repository consistency test](https://app.circleci.com/pipelines/github/huggingface/transformers/79103/workflows/fc012c2a-b047-4538-8f42-355d168ad5c2/jobs/1009858) is complaining that some docstrings don't match the object's signature:

Could you give me some pointers on this? I have been going around the docstrings for the two failing functions (I have fixed `SwiftformerConfig`).

Thank you!

Hi @joaocmd, to answer your first question, there's already a `safetensors` PR open for the model. I bumped the PR [here](https://huggingface.co/MBZUAI/swiftformer-xs/discussions/1) - hopefully we can get the repo owners to merge it.

As for the repository consistency test, have you tried running `make fix-copies` again? If it's still not working, let me know and I'll dig in deeper


Hi @Rocketknight1, I think that the command is not doing anything:
![imagem](https://github.com/huggingface/transformers/assets/5345834/011d5b4c-f142-4b5c-a2d3-9d57bfeb5822)

Should I take any other actions after running the command?

I admit I can't figure out the docstring issues here without a deeper dive - @amyeroberts do you have any ideas offhand? If not I'll get around to it in a day or two - we need to wait until the safetensors PR is merged anyway!

@joaocmd Not sure, unfortunately 😬  I'd suggest removing the `add_code_sample_docstrings` decorator for those models to test whether the docstrings are being built as the docstring checker expects. 

> @joaocmd Not sure, unfortunately 😬 I'd suggest removing the `add_code_sample_docstrings` decorator for those models to test whether the docstrings are being built as the docstring checker expects.

Hi @amyeroberts, just tested it and seems like I still get the same errors. Do you want me to push it?

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](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md) are likely to be ignored.

Hi @joaocmd, would you like us to re-open this PR - are you still interested in working on it? 

With regards to pushing changes - if the code is broken I wouldn't recommend pushing it, unless you want to save the changes for the history later. 

Hi @amyeroberts! I just noticed a few days ago that the safetensors PR was merged but I hadn't yet gotten around to testing it.

I would like to finish this issue. I've rerun the tests locally without `add_code_sample_docstrings` and it passed everything on my side.

@joaocmd OK - great! Glad everything's working :) Does this include the slow model tests? 

@Rocketknight1 Would you have time to help or look into the docstrings checks issue for the TF model? 

I'll try to find time to delve in @amyeroberts!

> @joaocmd OK - great! Glad everything's working :) Does this include the slow model tests?

Hi @amyeroberts, I used the command from the [documentation](https://huggingface.co/docs/transformers/add_tensorflow_model#6-7-ensure-everyone-can-use-your-model) which includes `RUN_SLOW=1`, I suppose that is what you mean?

After rebasing on main I get `FAILED tests/models/swiftformer/test_modeling_tf_swiftformer.py::TFSwiftFormerModelTest::test_save_load - AssertionError: 0.52219206 not less than or equal to 1e-05`. Any idea what might have caused this difference?

@joaocmd Yes, that's what I mean :) 

>After rebasing on main I get FAILED tests/models/swiftformer/test_modeling_tf_swiftformer.py::TFSwiftFormerModelTest::test_save_load - AssertionError: 0.52219206 not less than or equal to 1e-05. Any idea what might have caused this difference?

If in the `test_save_load` test you force avoiding safetensors: pass in `use_safetensors=False` in `from_pretrained` and `safe_serialization=False` in `save_pretrained` do you still get this result? 


Hi @amyeroberts, I tried doing that but I get this error:
```
FAILED tests/models/swiftformer/test_modeling_tf_swiftformer.py::TFSwiftFormerModelTest::test_save_load - TypeError: ('Keyword argument not understood:', 'use_safetensors')
```
I tried looking up the [documentation ](https://huggingface.co/docs/transformers/v4.36.1/en/main_classes/model#transformers.TFPreTrainedModel.from_pretrained) and source code for that function but I don't see anything regarding that parameter,

Hey, I was unable to reproduce the docstring problem locally (though digging around with it led me to fix a different docstring issue we have at #28450) - I'm adding some debug print statements to this PR so the CI tells us what's going on, and I'll remove them once I can figure things out.

@joaocmd Note that I did a rebase to help diagnose the issue and include #28450 - make sure you pull the latest changes before working on this branch locally!

@joaocmd We concluded that the docstring checker has some issues that are affecting this file, and your docstrings are actually fine. I've added the Swiftformer classes to the list of objects to be skipped by the checker, and the repo-consistency checks should be green for you now!

Hi @Rocketknight1, thank you! @amyeroberts did you see my last comment regarding the `use_safetensors` argument?

@joaocmd Indeed - it seems the `TFPreTrainedModel` doesn't accept `use_safetensors` in its `from_pretrained` method - which means its API deviates from the pytorch equivalent. @Rocketknight1 do you know why this is - is this deliberate or an oversight on our part aligning the two classes? 

Hi @amyeroberts - it wasn't deliberate! I can see if I can add that argument, one sec!

PR to add the argument is here: #28511

Added! @joaocmd you should be able to rebase to get the argument now.

> Added! @joaocmd you should be able to rebase to get the argument now.

Thanks @Rocketknight1! Will try it later today.

One further comment - I think the issue with the save-load test is because weights are not being built correctly. The reason for this is that we transitioned TF models to using full build methods in #27794. I'll add `build()` methods here as well using the script I used for that PR, which should hopefully resolve the issue.

Thanks @Rocketknight1, all checks passing now! I have changed the PR to ready for review. I also left a few `FIXME` in the model file and test files, some regarding the original SwiftFormer implementation and some others regarding the conversion. Thank you both for your effort!

@amyeroberts I think this should be ready for core maintainer review now! If you find any remaining TF issues, let me know and I'll dig in.

Hi @amyeroberts! I have addressed most of the comments and left some questions regarding some I'm not sure about yet. Could you take a look?

Hey @joaocmd - took a quick look and this seems pretty ready. I did some quick cleanup to get the repository consistency tests to pass, but everything else looks green. Were there other questions (besides the drop_path one) not addressed yet?

Hi @Rocketknight1! I think the only comment that was left unaddressed was this one: https://github.com/huggingface/transformers/pull/23342#discussion_r1468945311

All others seem to be addressed!


`# Copied from` has a specific meaning in our codebase - the CI will actually verify that the two functions are identical. If you've adapted the code at all (other than just renames like `BertConfig` -> `RobertaConfig` or something like that) then you should skip `# Copied from`.

Alright @Rocketknight1, then I believe it's ready to be merged! Thanks for your patience and help!

However, I just noticed it's failing on a documentation test but I don't understand the issue. Any idea what is the reason?

Hi @joaocmd - I think rebasing on the latest version of `main` should fix that one!

Also cc @amyeroberts for the final review - we should be fully green after that and I don't think there are any questions left.

Hey @Rocketknight1, seems like there are some new errors once again related to `test_pt_tf_model_equivalence` and `test_save_load`. Could you take a look?

Just realized that by rebasing I forgot to pull your commits first. I think my last three commits were similar to yours, sorry about that!

Hey @joaocmd - any idea what's up with the failing `test_save_load` test? I don't think it was failing before, and I pulled the branch locally and it passes for me! 

Hi @Rocketknight1, I don't know what might be the cause. I got the same errors locally when I was testing it. Also, the last time I remember seeing a related error was before merging the safe tensors were merged.

Hi @joaocmd - usually errors like that are an issue with tensor naming, as it suggests one of the weights isn't correctly being saved or loaded. If you can reproduce the error locally, can you try running the test with `--capture=no` and checking the log when the model is loaded to see if you can see the name(s) of the missing weight?

Hi @Rocketknight1, I can't find the root cause. I tried running with `capture=no` like you suggested but couldn't extract much from the output. I think this is the only relevant part I could take from it but it's not much different from a stack trace:

```
__________________________________________________________ TFSwiftFormerModelTest.test_save_load __________________________________________________________

self = <tests.models.swiftformer.test_modeling_tf_swiftformer.TFSwiftFormerModelTest testMethod=test_save_load>

    def test_save_load(self):
        config, inputs_dict = self.model_tester.prepare_config_and_inputs_for_common()
    
        for model_class in self.all_model_classes:
            model = model_class(config)
            outputs = model(self._prepare_for_class(inputs_dict, model_class))
    
            with tempfile.TemporaryDirectory() as tmpdirname:
                model.save_pretrained(tmpdirname, saved_model=False)
    
                # the config file (and the generation config file, if it can generate) should be saved
                self.assertTrue(os.path.exists(os.path.join(tmpdirname, CONFIG_NAME)))
                self.assertEqual(
                    model.can_generate(), os.path.exists(os.path.join(tmpdirname, GENERATION_CONFIG_NAME))
                )
    
                model = model_class.from_pretrained(tmpdirname)
                after_outputs = model(self._prepare_for_class(inputs_dict, model_class))
    
>               self.assert_outputs_same(after_outputs, outputs)

tests/test_modeling_tf_common.py:200: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/test_modeling_tf_common.py:421: in assert_outputs_same
    self.assertLessEqual(max_diff, 1e-5)
E   AssertionError: 0.4179657 not less than or equal to 1e-05
```

I also tried running the test with the debugger and exported the model weight names before and after loading from the saved model. All weights match except for the prefix, which I suppose is expected? The image below illustrates what I mean
![imagem](https://github.com/huggingface/transformers/assets/5345834/c61db50c-7cd5-4417-a8cb-4703fb290fa7)


Tried comparing the weight values directly and they seem to be very different. `w1` is the original weights and `w2` is after loading the saved model.
![imagem](https://github.com/huggingface/transformers/assets/5345834/99b9bd94-2b3c-4bb4-bc61-ceded1e8a713)
![imagem](https://github.com/huggingface/transformers/assets/5345834/2725e037-7718-4cff-a462-a7a0995f6966)



Hey @Rocketknight1, did you see my last two comments?

Ahh, I missed them, I'm sorry! If I'm ever being slow like that again, feel free to ping me - there's just a lot going on in `transformers` right now. I'll pull your branch now and see if I can figure it out.

@joaocmd I think it's fixed! I'm not sure what was causing the error initially, but I suspect it was related to Keras incompatibilities. We changed how we handle Keras imports with the arrival of Keras 3, and changed all of our code to import the right `keras` from `modeling_tf_utils`. Making the same update in your code fixed the issue. There's one test failure remaining, but I think that can be fixed just by rebasing.

Hey @Rocketknight1! Just rebased it, seems like there is still some issue unrelated to this PR

Yeah, that's on our end - it'll be fixed soon, and at that point we should be good to merge. I think you're almost done though - I'll try to do a final review on this today or Monday, and then we'll get a review from a core maintainer and we should be good to merge!

Actually, forgot @amyeroberts has already done a core maintainer pass! @amyeroberts are we good to merge once we can rebase and fix the last (unrelated) test failure?

@Rocketknight1 I'll need to do another review - I didn't approve in the last one and that was a long time (and many commits) ago. Once you've done your final review ping me and I'll review again : )

Just finished my review - the port looks really good, with one comment! Also note that if you rebase on `main` now the remaining test issues should finally be fixed!

Hi @Rocketknight1! I've accepted your suggestions and left a comment regarding your question.

Yes, this looks good now! I left one more comment, but it's a nit at this point, I think we're basically ready and all tests are green. cc @amyeroberts for final core maintainer review!

Hi @joaocmd, I think the problem is that the `from transformers.modeling_tf_utils import keras` line is not in the `if is_tf_available()` block - this means it's getting imported in non-TF tests that don't have the required TF dependencies!

Thanks @Rocketknight1, I think all is set now!

Yes, cc @amyeroberts for final review, I think we're ready!

Regarding this comment: https://github.com/huggingface/transformers/pull/23342#discussion_r1536065082, I tried reducing the `image_size` by half but the tests started failing. I believe it's due to how the model expects a specific number of patch embeddings, but I could be wrong. Do you think it's fine if we keep it as it is @amyeroberts?

@joaocmd OK, if it's a pain to change and other params like the batch size are reduced it should be OK

Hi @amyeroberts I think this is the last unaddressed comment: https://github.com/huggingface/transformers/pull/23342#discussion_r1536064298

Could you remind me what the problem was? I didn't find it. Thanks!

@joaocmd [Original comment here](https://github.com/huggingface/transformers/pull/23342/files#r1524732572)

Hi @amyeroberts! I think I've addressed all the comments. The CI pipeline is failing on a `ruff` check even though I've run it locally. Do you know why?

@joaocmd It might be a version mismatch. To ensure you have the same versions of libraries like ruff in your env, you can run `pip install -e .[quality]`. I'd then try and run `make fixup` again 

Thanks @amyeroberts! Is there anything missing on my side?

Thank you @amyeroberts and @Rocketknight1 for your patience and support! It was a pleasure ☺️