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

Scope onnx-simplifier requirements check #4730

Merged
merged 2 commits into from
Sep 9, 2021

Conversation

Zegorax
Copy link
Contributor

@Zegorax Zegorax commented Sep 9, 2021

Export.py has been updated to check for onnx-simplifier requirement only when the --simplify argument is added.
Allows for better flexibility and one less requirement if simplify is not needed.

๐Ÿ› ๏ธ PR Summary

Made with โค๏ธ by Ultralytics Actions

๐ŸŒŸ Summary

Improvements to ONNX export dependencies management in YOLOv5.

๐Ÿ“Š Key Changes

  • Removed onnx-simplifier from the immediate dependency check during ONNX export.
  • Added a separate dependency check for onnx-simplifier only when simplification is required.

๐ŸŽฏ Purpose & Impact

  • Purpose: To streamline the export process by only requiring necessary dependencies, avoiding the loading of extra packages unless needed.
  • Impact: Users who export to ONNX without the need for simplification will experience a faster setup as the simplifier package won't be checked or loaded initially. This change could reduce the initial overhead, making the export process slightly quicker and more efficient for those not using simplification. ๐Ÿš€

Export.py has been updated to check for onnx-simplifier requirement only when the --simplify argument is added.
Allows for better flexibility and one less requirement if simplify is not needed.
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

๐Ÿ‘‹ Hello @Zegorax, thank you for submitting a ๐Ÿš€ PR! To allow your work to be integrated as seamlessly as possible, we advise you to:

  • โœ… Verify your PR is up-to-date with origin/master. If your PR is behind origin/master an automatic GitHub actions rebase may be attempted by including the /rebase command in a comment body, or by running the following code, replacing 'feature' with the name of your local branch:
git remote add upstream https://github.com/ultralytics/yolov5.git
git fetch upstream
git checkout feature  # <----- replace 'feature' with local branch name
git rebase upstream/master
git push -u origin -f
  • โœ… Verify all Continuous Integration (CI) checks are passing.
  • โœ… Reduce changes to the absolute minimum required for your bug fix or feature addition. "It is not daily increase but daily decrease, hack away the unessential. The closer to the source, the less wastage there is." -Bruce Lee

@glenn-jocher glenn-jocher changed the title Changed onnx-simplifier check behavior Scope onnx-simplifier requirements check Sep 9, 2021
@glenn-jocher glenn-jocher merged commit 2d9411d into ultralytics:master Sep 9, 2021
@glenn-jocher
Copy link
Member

@Zegorax PR is merged. Thank you for your contributions to YOLOv5 ๐Ÿš€ and Vision AI โญ

@glenn-jocher
Copy link
Member

@Zegorax was thinking about this, it's a bit of a mixed result. If a user does not --simplify then we've improved things for them, if they do --simplify then we've doubled the calls to check_requirements(), and it will report twice independently as well, i.e. (1 package updated, 1 package updated) rather than combined (2 packages updated).

@Zegorax
Copy link
Contributor Author

Zegorax commented Sep 9, 2021

@glenn-jocher Yes, thanks a lot for the merge. I believe it's better like this and doesn't force to have onnx-simplifier if it is not needed.

CesarBazanAV pushed a commit to CesarBazanAV/yolov5 that referenced this pull request Sep 29, 2021
* Changed onnx-simplifier check behavior

Export.py has been updated to check for onnx-simplifier requirement only when the --simplify argument is added.
Allows for better flexibility and one less requirement if simplify is not needed.

* Fix single-element tuples

Co-authored-by: Glenn Jocher <glenn.jocher@ultralytics.com>
BjarneKuehl pushed a commit to fhkiel-mlaip/yolov5 that referenced this pull request Aug 26, 2022
* Changed onnx-simplifier check behavior

Export.py has been updated to check for onnx-simplifier requirement only when the --simplify argument is added.
Allows for better flexibility and one less requirement if simplify is not needed.

* Fix single-element tuples

Co-authored-by: Glenn Jocher <glenn.jocher@ultralytics.com>
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

2 participants