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

fix compatibility for hyper config #1146

Merged
merged 5 commits into from
Oct 15, 2020
Merged

Conversation

Borda
Copy link
Contributor

@Borda Borda commented Oct 15, 2020

changes in #1120 renamed a parameter in hyper config which means that all past configs are failing and from the error it is not very clear, so this is adding explanation warning and if the new argument is missing it uses the old one...

🛠️ PR Summary

Made with ❤️ by Ultralytics Actions

🌟 Summary

Improved compatibility and refactored device selection in YOLOv5 training script.

📊 Key Changes

  • Moved device selection to support Distributed Data Parallel (DDP) mode more seamlessly.
  • Added a warning to handle missing "box" hyperparameter in configs, providing backward compatibility.
  • Reordered imports for code cleanliness and standards.
  • Refactored parts of the code to enhance maintainability and readability.

🎯 Purpose & Impact

  • 🔁 The refactoring around device selection is meant to better support DDP, allowing for more scalable and efficient training across multiple GPUs 🚀.
  • ⚠️ Warning for missing "box" hyperparameter ensures that users with older configurations will get notified about the change, preventing potential training issues 😌.
  • ✨ With the overall refactoring and import reordering, the code base becomes cleaner, which makes it easier for developers to navigate and contribute 🧹.
  • These changes should not directly affect model performance but can improve the user experience and reduce bugs in future developments 🛠️.

@Borda
Copy link
Contributor Author

Borda commented Oct 15, 2020

@glenn-jocher can we merge this compatibility hotfix 🐰

@glenn-jocher
Copy link
Member

@Borda that's a good idea. Do you think we should actually place this in train.py right after the hyp.yaml is loaded? This would allow us to specify the exact name of the hyp file as well, i.e. 'problem with %s' % opt.hyp

@glenn-jocher

This comment has been minimized.

@glenn-jocher
Copy link
Member

glenn-jocher commented Oct 15, 2020

Ah, actually this combines the renaming and deleting into a single line:

if 'box' not in h:
        warn(f'Compatibility: {opt.hyp} missing "box" which was renamed from past "giou" in https://github.com/ultralytics/yolov5/pull/1120')
        h['box'] = h.pop('giou')

@Borda
Copy link
Contributor Author

Borda commented Oct 15, 2020

@Borda that's a good idea. Do you think we should actually place this in train.py right after the hyp.yaml is loaded? This would allow us to specify the exact name of the hyp file as well, i.e. 'problem with %s' % opt.hyp

is the Trainer the only place from where the hyper can come? if you then train would be better...

@glenn-jocher
Copy link
Member

Ok, I've sent this to train.py right after hyps are loaded (its the only place they are loaded), and tested. Output shows this, training starts correctly.

/Users/glennjocher/PycharmProjects/yolov5/train.py:448: UserWarning: Compatibility: data/hyp.scratch.yaml missing "box" which was renamed from "giou" in https://github.com/ultralytics/yolov5/pull/1120
  warn('Compatibility: %s missing "box" which was renamed from "giou" in %s' %

@glenn-jocher glenn-jocher merged commit c67e722 into ultralytics:master Oct 15, 2020
@Borda Borda deleted the fix/hyper branch October 15, 2020 13:25
glenn-jocher added a commit that referenced this pull request Oct 15, 2020
* fix/hyper

* Hyp giou check to train.py

* restore general.py

* train.py overwrite fix

* restore general.py and pep8 update

Co-authored-by: Glenn Jocher <glenn.jocher@ultralytics.com>
burglarhobbit pushed a commit to burglarhobbit/yolov5 that referenced this pull request Jan 1, 2021
* fix/hyper

* Hyp giou check to train.py

* restore general.py

* train.py overwrite fix

* restore general.py and pep8 update

Co-authored-by: Glenn Jocher <glenn.jocher@ultralytics.com>
KMint1819 pushed a commit to KMint1819/yolov5 that referenced this pull request May 12, 2021
* fix/hyper

* Hyp giou check to train.py

* restore general.py

* train.py overwrite fix

* restore general.py and pep8 update

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
* fix/hyper

* Hyp giou check to train.py

* restore general.py

* train.py overwrite fix

* restore general.py and pep8 update

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.

2 participants