Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pytorch2 compile (#1505) #1516

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

N-Friederich
Copy link
Contributor

@N-Friederich N-Friederich commented Mar 20, 2023

Extension accordingly #1505

  • Add pytorch.compile(...) to Yolo.init(...) incl. version check
  • Use Compile with default options. More information here

@AyushExel @glenn-jocher Maybe there is a better place. We'll have to discuss that. Currently, I see a significant speedup of 1.5 on Apple M2 Pro with MPS. Sorry, I am still in the process of understanding the code and the structure of the code modules.

πŸ€– Generated by Copilot at e984a48

Summary

πŸš€πŸ› οΈπŸ“¦

This pull request adds an option to compile the YOLO model with the experimental PyTorch 2.0 feature, which improves the performance but prevents exporting. It also updates the tests, benchmarks, and exporter to disable this feature by default, to ensure compatibility and stability.

We're sailing with the YOLO crew, we've got a job to do
We need to export our models fast, but PyTorch 2 won't do
So we'll pass a flag to skip the compile, and save ourselves some trouble
Heave away, me hearties, heave away on the double

Walkthrough

  • Add compile_model argument to YOLO constructor and check PyTorch version and compile model with experimental feature if True (link, link, link)
  • Raise ValueError in YOLO.export method if compile_model is True (link)
  • Pass compile_model=False to YOLO constructor in export function in ultralytics/yolo/engine/exporter.py and benchmark function in ultralytics/yolo/utils/benchmarks.py (link, link, link)
  • Pass compile_model argument to YOLO constructor based on mode argument in entrypoint function in ultralytics/yolo/cfg/__init__.py (link)
  • Modify test functions for exporting model to different formats to pass compile_model=False to YOLO constructor in tests/test_python.py (link, link, link, link)
  • Import check_version function from ultralytics/yolo/utils/checks.py to ultralytics/yolo/engine/model.py and ultralytics/yolo/engine/trainer.py (link, link)
  • Import torch to ultralytics/yolo/engine/model.py and remove unused import from ultralytics/yolo/cfg/__init__.py (link, link)

πŸ› οΈ PR Summary

Made with ❀️ by Ultralytics Actions

πŸ“Š Key Changes

  • Added compile_model flag to allow the compilation of PyTorch models with new Torch 2.0 features.
  • Adjusted model property in the trainer.py to use the compiled model if the compile_model flag is true and the PyTorch version is 2.0 or above.
  • Updated various parts of the codebase to support this new compilation feature including autobackend.py, predictor.py, and default.yaml.

🎯 Purpose & Impact

  • The goal of these changes is to utilize the performance benefits provided by model compilation in PyTorch 2.0, potentially reducing inference time and improving efficiency.
  • Users working with the updated Ultralytics repository can now expect faster model execution if using PyTorch 2.0, given they enable the new compile_model option.

🌟 Summary

Leverage PyTorch 2.0's model compilation for enhanced performance in Ultralytics' AI models. πŸš€

@glenn-jocher
Copy link
Member

@N-Friederich got it. It looks like most CI is passing except for exports.

@glenn-jocher
Copy link
Member

Also note Python 1.7 is not supported by torch 2.0, which is why 1.7 CI is passing.

@glenn-jocher glenn-jocher changed the base branch from main to updates March 21, 2023 11:24
@glenn-jocher glenn-jocher changed the base branch from updates to main March 21, 2023 11:24
@N-Friederich
Copy link
Contributor Author

N-Friederich commented Mar 21, 2023

Also note Python 1.7 is not supported by torch 2.0, which is why 1.7 CI is passing.

@glenn-jocher Good point. I'll take a look.

@N-Friederich
Copy link
Contributor Author

N-Friederich commented Mar 21, 2023

Also note Python 1.7 is not supported by torch 2.0, which is why 1.7 CI is passing.

@glenn-jocher Ok, but then this is a pytorch recommendation. Maybe we need to build a test case explicitly for Pytorch 2?

@glenn-jocher
Copy link
Member

No don't worry, CI is fine, since 1.13.1 is applied automatically for Python 1.7 which will end of life in a few months, all over CI's use torch 2.0 and are passing in main branch now.

The problem is the compiled models don't seem to export correctly. Maybe we should only compile when an inference command is run, i.e. predict, train, val?

@N-Friederich
Copy link
Contributor Author

@glenn-jocher I think a torch._dynamo.reset() do the job in the export method. I will test this later.

@N-Friederich
Copy link
Contributor Author

@glenn-jocher I think I misunderstood the method. I am currently considering an alternative.

@N-Friederich
Copy link
Contributor Author

@glenn-jocher I think the problem is difficult to solve. Because what if you call train(...) in python and then export(...) on the same model. if this is compiled, you can no longer export it.

@N-Friederich
Copy link
Contributor Author

@glenn-jocher I'll look at it again in detail tomorrow.

@glenn-jocher
Copy link
Member

@glenn-jocher I think the problem is difficult to solve. Because what if you call train(...) in python and then export(...) on the same model. if this is compiled, you can no longer export it.

Yes might be a little complicated, but I think the first step would be to only compile on inference methods (predict, val, train), and then check in export if compiled: warning with explanation and exit.

@glenn-jocher
Copy link
Member

@N-Friederich I reviewed https://pytorch.org/tutorials/intermediate/torch_compile_tutorial.html, it seems that compile ops are best used for long term actions like training and val on large datasets. Their demo code for example shows that it takes several seconds to compile a model, so it may not be suitable for simply running inference on an image.

Maybe we should opt-in with an argument to let the user decide, i.e. compile=False?

@AyushExel @Laughing-q what do you guys think of the new torch 2.0 compile examples in the link above?

@glenn-jocher
Copy link
Member

@N-Friederich I tried to compile on train, but getting compile errors. Seems like it may not be ready yet to apply by default, probably want an opt-in argument here.

@N-Friederich
Copy link
Contributor Author

N-Friederich commented Mar 22, 2023

@glenn-jocher I think the problem is difficult to solve. Because what if you call train(...) in python and then export(...) on the same model. if this is compiled, you can no longer export it.

Yes might be a little complicated, but I think the first step would be to only compile on inference methods (predict, val, train), and then check in export if compiled: warning with explanation and exit.

@glenn-jocher I have never received an error. Tested it for a Titan RTX and MPS (+CPU).Can you send me your error?

@N-Friederich
Copy link
Contributor Author

N-Friederich commented Mar 22, 2023

@N-Friederich I tried to compile on train, but getting compile errors. Seems like it may not be ready yet to apply by default, probably want an opt-in argument here.

@glenn-jocher That was my idea too. But I think it's a bit "ugly". That's why I wanted to build something better. But I think that is now the only option.

@AyushExel
Copy link
Contributor

Yeah I agree with having compile=True opt in

@N-Friederich
Copy link
Contributor Author

@glenn-jocher @AyushExel 1) The question is how we deal with benchmark. Because pytorch models should be supported. 2.) If export is used, only a warning and skipping, or an error? In case of an error, the tests would have to be adapted accordingly.

- Set compile=False for all export operations
…pile

# Conflicts:
#	ultralytics/yolo/engine/model.py
@N-Friederich
Copy link
Contributor Author

N-Friederich commented Mar 23, 2023

@glenn-jocher @AyushExel I have implemented it for now without any additional flag and only turned it off for export. Do you think we also need a flag?
By the way, the tutorials also need to be adjusted in the Jupyter. I have already adjusted the tests.

@glenn-jocher
Copy link
Member

@N-Friederich got it! Please fix the CI errors that are appearing.

@N-Friederich
Copy link
Contributor Author

@glenn-jocher This is interesting. I tested it locally before my commit and got no errors. I'll take a look at it.

@N-Friederich
Copy link
Contributor Author

N-Friederich commented Mar 24, 2023

@glenn-jocher The problem is that the model is saved as a pickel. This is generally not recommended and leads to the error with compiled models. I have now changed the saving as recommended here. But their are some issues with weight loading.

@glenn-jocher
Copy link
Member

@N-Friederich I haven't looked too deeply into the changes but I see you've added a manual argument on model load, which we want to avoid. We infer a model type in AutoBackend and then naturally don't run compile commands if type is not PyTorch.

YOLO() should be able to load and handle all export formats without any additional arguments required.

@N-Friederich
Copy link
Contributor Author

@glenn-jocher The problem currently is rather the serialization. This is currently not possible directly from a compiled model. I.e. there must be considered an alternative. But I am not as deep in the code as you are.
image
(Source: https://pytorch.org/get-started/pytorch-2.0/)

@N-Friederich
Copy link
Contributor Author

@N-Friederich I haven't looked too deeply into the changes but I see you've added a manual argument on model load, which we want to avoid. We infer a model type in AutoBackend and then naturally don't run compile commands if type is not PyTorch.

YOLO() should be able to load and handle all export formats without any additional arguments required.

@glenn-jocher I just changed something in the YOLO.init(...), is that what you meant by "model load"?

@glenn-jocher
Copy link
Member

@N-Friederich yes, I meant the YOLO() initialization. You added a manual argument for model load, which we should try to avoid. Instead, we should infer the model type in AutoBackend to determine whether or not to run compile commands. Additionally, YOLO() should be able to load and handle all export formats without needing additional arguments.

@N-Friederich
Copy link
Contributor Author

@N-Friederich yes, I meant the YOLO() initialization. You added a manual argument for model load, which we should try to avoid. Instead, we should infer the model type in AutoBackend to determine whether or not to run compile commands. Additionally, YOLO() should be able to load and handle all export formats without needing additional arguments.

@glenn-jocher Ok, I'll take a look. Thanks for the info.

@glenn-jocher
Copy link
Member

@N-Friederich sure, I'd be happy to explain ideas and concepts without using code examples. Let me know what you need help with!

@N-Friederich
Copy link
Contributor Author

@glenn-jocher From my perspective, there are several ways to resolve the problem. One could (similar to EMA) save a separate model for the compiled model and transfer the weights to the non-compiled model before saving. This is the best option from my perspective because as of now you can't serialize a compiled model directly (except half via the state_dict).

# Conflicts:
#	ultralytics/yolo/cfg/__init__.py
@N-Friederich
Copy link
Contributor Author

@glenn-jocher @AyushExel I think I have found a relatively good way. Have implemented only for train for now, should we do it for the Val and Pred as well? I would say, if then we should compile the model before. Then the compile time is not included.
And srry that it took so long, I think I understand the code better now (The changes from YOLOv5 to YOLOv8 was bigger than I thought) :)

@glenn-jocher
Copy link
Member

@N-Friederich That's great to hear that you have found a solution to the problem of serializing compiled models. It's important to note that the solution should be scaleable and applicable for all stages of the pipeline, which means that it should be implemented for Val and Pred as well. If the model needs to be compiled before the serialization, then we should include the compile time in the overall processing time as well. It's understandable that it took a while to familiarize yourself with the changes from YOLOv5 to YOLOv8, but taking the time to understand the code is necessary to ensure the quality of the contributions.

@nodeav
Copy link
Contributor

nodeav commented Jun 12, 2023

@N-Friederich @glenn-jocher Hey! is there any way to push this forward?
I'd be more than happy to chip in and lend a hand if it makes a difference.

@N-Friederich
Copy link
Contributor Author

Hi @nodeav ,
thanks for your message! I had not tried since then. Unfortunately, it's not that easy, especially regarding the model export it's really tricky. I think the best way would be to do it with a decorator, but then some exports will fail.
Do you have ideas @glenn-jocher and @glenn-jocher ? Think this is a great-added value for YOLOv8.
Thx and best,
Nils

@nodeav
Copy link
Contributor

nodeav commented Jun 22, 2023

I think if it's only done for training purposes (for now?), it might be simpler? then export the non-compiled version, because pytorch doesn't support it yet AFAIK

@pderrenger
Copy link
Member

@nodeav absolutely, focusing on implementing this specifically for training purposes initially would simplify the process. After training, the non-compiled version can be exported, considering that PyTorch doesn't currently fully support exporting compiled models. This iterative approach maintains simplicity and can be expanded upon as the ecosystem evolves.

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.

None yet

5 participants