fix(DPT,Depth-Anything) `torch.export`
Author: philkuz
Pull request: yes
Labels: name: Vision

name: run-slow

name: ExecuTorch

# What does this PR do?
Small modification of the DPT modeling code to remove a new object creation in a `forward()` method of a Module. This object creation makes the model incompatible with `torch.export`, which is a key part of preparing a model to run on a variety of hardware backends through projects such as [ExecuTorch](https://pytorch.org/executorch/main/intro-overview.html) (related issue: https://github.com/huggingface/transformers/issues/32253)

## Motivation
[torch.export](https://pytorch.org/tutorials/intermediate/torch_export_tutorial.html#) allows you to export PyTorch models into standardized model representations, intended to be optimized and run efficiently using frameworks such as TensorRT or ExecuTorch.

## The Bug 
They key issue was the slice on `self.layers`:
https://github.com/huggingface/transformers/blob/617b21273a349bd3a94e2b3bfb83f8089f45749b/src/transformers/models/dpt/modeling_dpt.py#L696

`self.layers[1:]` creates a new `ModuleList()` each time this line is executed.

https://github.com/pytorch/pytorch/blob/69bcf1035e7f06f2eefd8986d000cc980e9ebd37/torch/nn/modules/container.py#L330

The model tracer in `torch.export` monkey-patches nn.Module constructors during evaluation of the `forward()` pass, so the original DPT modeling code raises the following error:
```
  File "/home/philkuz/.pyenv/versions/gml311/lib/python3.11/site-packages/torch/nn/modules/container.py", line 293, in __getitem__
      return self.__class__(list(self._modules.values())[idx])
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                 TypeError: _ModuleStackTracer.__init__.<locals>.AttrProxy.__init__() missing 1 required positional argument: 'path'

```

## The Solution
Pytorch recommends users update the modeling code. My team and I figured this could be helpful to the broader community, especially a future where Export to Executorch becomes more widely available: https://github.com/huggingface/transformers/issues/32253

This also removes an unnecessary creation of a new module list as a bonus.

### Tests
I ensured that `tests/models/dpt/test_modeling_dpt.py` passes, which appears to test a portion of the outputs. I also verified that the entire output of the model
before and after my changes matched with the following script:
```python
import os
import sys

import numpy as np
import requests
import torch
from PIL import Image
from transformers import pipeline

url = "http://images.cocodataset.org/val2017/000000039769.jpg"
image = Image.open(requests.get(url, stream=True).raw)


model = pipeline("depth-estimation", "facebook/dpt-dinov2-base-kitti")
result = model(image)


output_file = "depth_estimation_output.npy"

if not os.path.exists(output_file):
    # Save the current output
    np.save(output_file, result["predicted_depth"])
    print(f"Depth estimation output saved to {output_file}")
    print("Rerun the script to compare the output")
    sys.exit(0)
# Load existing output and compare
expected_output = np.load(output_file)
np.testing.assert_allclose(
    result["predicted_depth"],
    expected_output,
    rtol=1e-5,
    atol=1e-5,
    err_msg="Depth estimation output has changed",
)
print("Depth estimation output matches the saved version.")
```

<!--
Congratulations! You've made it this far! You're not quite done yet though.

Once merged, your PR is going to appear in the release notes with the title you set, so make sure it's a great title that fully reflects the extent of your awesome contribution.

Then, please replace this with a description of the change and which issue is fixed (if applicable). Please also include relevant motivation and context. List any dependencies (if any) that are required for this change.

Once you're done, someone will review your PR shortly (see the section "Who can review?" below to tag some potential reviewers). They may suggest changes to make the code even better. If no one reviewed your PR after a week has passed, don't hesitate to post a new comment @-mentioning the same persons---sometimes notifications get lost.
-->

<!-- Remove if not applicable -->


## 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#create-a-pull-request),
      Pull Request section?
- [ ] 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.
- [ ] 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).
- [ ] 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, @qubvel

<!-- Your PR will be replied to more quickly if you can figure out the right person to tag with @

 If you know how to use git blame, that is the easiest way, otherwise, here is a rough guide of **who to tag**.
 Please tag fewer than 3 people.

Models:

- text models: @ArthurZucker
- speech models: @ylacombe, @eustlb
- graph models: @clefourrier

Library:

- flax: @sanchit-gandhi
- generate: @zucchini-nlp (visual-language models) or @gante (all others)
- pipelines: @Rocketknight1
- tensorflow: @gante and @Rocketknight1
- tokenizers: @ArthurZucker
- trainer: @muellerzr and @SunMarc
- chat templates: @Rocketknight1

Integrations:

- deepspeed: HF Trainer/Accelerate: @muellerzr
- ray/raytune: @richardliaw, @amogkam
- Big Model Inference: @SunMarc
- quantization (bitsandbytes, autogpt): @SunMarc

Documentation: @stevhliu

HF projects:

- accelerate: [different repo](https://github.com/huggingface/accelerate)
- datasets: [different repo](https://github.com/huggingface/datasets)
- diffusers: [different repo](https://github.com/huggingface/diffusers)
- rust tokenizers: [different repo](https://github.com/huggingface/tokenizers)

Maintained examples (not research project or legacy):

- Flax: @sanchit-gandhi
- PyTorch: See Models above and tag the person corresponding to the modality of the example.
- TensorFlow: @Rocketknight1

 -->


Comments:
> Very nice, thanks for unlocking more models for torch export, this is very valuable!
> 
> The same comment as for Mask2Former PR, would be great to have this PR tested, and please push run-slow commit to trigger all tests at the end!

I've added Depth-anything to this PR, I'm not entirely sure if I've triggered the run_slow test for it and DPT correctly. Happy to split it off into a separate PR.

Also ran into an issue with zoedepth not working because of the beit backend. I suspect that will take more time to properly addres, I added a skipped test to zoedepth, but I can also remove that test entirely and add it in a WIP PR.


> Thanks for the update! Regarding ZoeDepth I suggest either fixing the model export or excluding this model from the PR. A skipped test is not the best solution, cause it might stuck in this state for a very long time 😄
> 
> To trigger multiple models' slow tests you can list them as follows `[run_slow] depth_anything, dpt, zoedepth`

I have to add some of the model changes because of the copy-consistency check, but I'll remove the Relu change and the torch.export test!

Thanks for the heads up on slow tests.

@qubvel could you approve the slow workflow?

> I have to add some of the model changes because of the copy-consistency check
Can you provide a bit more details on this? Can I somehow help to enable torch export for ZoeDepth?

> > I have to add some of the model changes because of the copy-consistency check
> Can you provide a bit more details on this? Can I somehow help to enable torch export for ZoeDepth?

I'm not 100% sure that this is part of the CI, but the contributing guide asks you to run repo-consistency 
https://github.com/huggingface/transformers/blob/bc0633a82cbfe8d828fa2d3b432dfde4fbd2f0e5/CONTRIBUTING.md#L217

which throws an error in  `python utils/check_copies.py` if you don't update ZoeDepth to match DPT. (ZoeDepth copied many layers from DPT
https://github.com/philkuz/transformers/blob/bc0633a82cbfe8d828fa2d3b432dfde4fbd2f0e5/src/transformers/models/zoedepth/modeling_zoedepth.py#L175
)

So basically I have to include those shared changes: https://github.com/huggingface/transformers/pull/34103/files#diff-02337c86e3fba49173cf2cb6fa1595ed168db19726938aec925b8b010a3b6a8c

The current crux of ZoeDepth is that the BEIT model, the backbone of all the HF hub models for ZoeDepth, isn't compatible. So you have to address that issue, which I have not had time to address yet.


The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_34103). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

The `slow_tests` are failing, but I think they're broken on main as well. Here's a repro:
```
git checkout main
# DPT Failures
CUDA_VISIBLE_DEVICES="" RUN_SLOW=true pytest tests/models/dpt/test_modeling_dpt_auto_backbone.py -v  -k=test_inference_depth_estimation_dinov2
# Depth-anything failures
CUDA_VISIBLE_DEVICES="" RUN_SLOW=true pytest tests/models/depth_anything/test_modeling_depth_anything.py -v  -k test_inference
```

The following also has checks on the output of slices and those checks seem to work.
```
CUDA_VISIBLE_DEVICES="" RUN_SLOW=true pytest tests/models/dpt/test_modeling_dpt.py -v  -k=test_inference
```

I went ahead and made a PR to try and address this issue: https://github.com/huggingface/transformers/pull/34518


> I'm not 100% sure that this is part of the CI, but the contributing guide asks you to run repo-consistency

Ahh, ok, it's because model modules contain "Copied from" statements and these parts are synced across models. No worries then!

Extend the script in https://github.com/pytorch/executorch/pull/6509 to support lowering (with the simplest recipe) the `DepthEstimation` and `SemanticSegmentation` models enabled in this PR. 

The `dpt` model works as expected. 
However, the `depth-anything` model fails due to an unsupported dim order. ExecuTorch supports `dim_order: (0, 1, 2, 3)` but got `dim_order: (0, 2, 3, 1)` for a placeholder node `aten_clone_default`. There seems to be a way to insert a compiler pass to fix it w/o requiring changing the source code. I will give a try.

> Extend the script in [pytorch/executorch#6509](https://github.com/pytorch/executorch/pull/6509) to support lowering (with the simplest recipe) the `DepthEstimation` and `SemanticSegmentation` models enabled in this PR.
> 
> The `dpt` model works as expected. However, the `depth-anything` model fails due to an unsupported dim order. ExecuTorch supports `dim_order: (0, 1, 2, 3)` but got `dim_order: (0, 2, 3, 1)` for a placeholder node `aten_clone_default`. There seems to be a way to insert a compiler pass to fix it w/o requiring changing the source code. I will give a try.

Any luck on the compiler pass? 

Also do you think that this gates the support for `torch.export`? Seems like this is `Executorch` specific. Maybe we can scope this PR down to be more for `torch.export` generally and focus on adding support for Executorch in another PR? Happy to help with that

> > Extend the script in [pytorch/executorch#6509](https://github.com/pytorch/executorch/pull/6509) to support lowering (with the simplest recipe) the `DepthEstimation` and `SemanticSegmentation` models enabled in this PR.
> > The `dpt` model works as expected. However, the `depth-anything` model fails due to an unsupported dim order. ExecuTorch supports `dim_order: (0, 1, 2, 3)` but got `dim_order: (0, 2, 3, 1)` for a placeholder node `aten_clone_default`. There seems to be a way to insert a compiler pass to fix it w/o requiring changing the source code. I will give a try.
> 
> Any luck on the compiler pass?

@philkuz Sorry, haven't got a chance to write the pass yet.

> 
> Also do you think that this gates the support for `torch.export`? Seems like this is `Executorch` specific. Maybe we can scope this PR down to be more for `torch.export` generally and focus on adding support for Executorch in another PR? Happy to help with that

Right, it's ExecuTorch specific, i.e. all tensors need to be contiguous. BTW, do you happen to know where the channel_last tensor may come from the eager, we can fix it here, otherwise, having a separate PR for ExecuTorch is fine. Please note that unlike compiled artifact the exported program is just an intermediate representation, typically should only being used as the entry for further optimizations, i.e. ExecuTorch. 

> > > Extend the script in [pytorch/executorch#6509](https://github.com/pytorch/executorch/pull/6509) to support lowering (with the simplest recipe) the `DepthEstimation` and `SemanticSegmentation` models enabled in this PR.
> > > The `dpt` model works as expected. However, the `depth-anything` model fails due to an unsupported dim order. ExecuTorch supports `dim_order: (0, 1, 2, 3)` but got `dim_order: (0, 2, 3, 1)` for a placeholder node `aten_clone_default`. There seems to be a way to insert a compiler pass to fix it w/o requiring changing the source code. I will give a try.
> > 
> > 
> > Any luck on the compiler pass?
> 
> @philkuz Sorry, haven't got a chance to write the pass yet.
> 
> > Also do you think that this gates the support for `torch.export`? Seems like this is `Executorch` specific. Maybe we can scope this PR down to be more for `torch.export` generally and focus on adding support for Executorch in another PR? Happy to help with that
> 
> Right, it's ExecuTorch specific, i.e. all tensors need to be contiguous. BTW, do you happen to know where the channel_last tensor may come from the eager, we can fix it here, otherwise, having a separate PR for ExecuTorch is fine. Please note that unlike compiled artifact the exported program is just an intermediate representation, typically should only being used as the entry for further optimizations, i.e. ExecuTorch.

I did a very quick scan for the channel_last tensor, and I believe it's in DINOv2 (the backbone) which is not part of this particular modeling code. I think we should move it to another PR IMO.

Any other block for merging this PR?

@guangy10 no blocks IMO, waiting for @ArthurZucker's review, he has quite a few in line 

Sorry for the delay @guangy10 we were on a company wide offsite! 🌴