add GH action CPU tests #426

Borda · 2020-07-16T07:37:16Z

Adding CPU CI testing using GH action, native to Github described in #407

🛠️ PR Summary

_{Made with ❤️ by Ultralytics Actions}

🌟 Summary

Added CI testing for CPU and tweaked weights download script.

📊 Key Changes

🆕 .gitattributes added to exclude notebooks from GitHub language stats.
🚀 New GitHub Actions workflow (ci-testing.yml) for Continuous Integration (CI) testing on CPUs.
🛠️ Updated README with a CI CPU testing badge to reflect the new GitHub Actions status.
🔄 Modified the weights/download_weights.sh script for downloading weight files with an updated syntax.

🎯 Purpose & Impact

🧹 Maintainability: By excluding notebooks from language stats, the repository's language breakdown on GitHub more accurately reflects the actual code base.
✅ Reliability: Introducing CI testing ensures code changes don't break functionality by automatically running tests across different Python versions and YOLOv5 models on CPU environments. This catches bugs early on.
📈 Transparency: Showing the CI testing badge in the README gives users and contributors a quick view of the current build status, increasing trust and clarity.
📦 Ease of Use: Streamlining the weight download process helps users and developers set up the environment more consistently, ensuring smooth operation.

These changes collectively aim to improve the project's code quality control, setup experience, and overall reliability for users and contributors.

Borda · 2020-07-16T10:23:43Z

@glenn-jocher @dlawrences mind checking this one...

Borda · 2020-07-16T11:52:29Z

maybe just one Python version for xOS as there are only 5 concurrency jobs in free version
https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#usage-limits

glenn-jocher · 2020-07-16T16:35:38Z

@Borda ah thank you! This looks really interesting. So this will run tests on cpu, which is a good start! Could we do this without renaming test.py and by leaving coco128 next to yolov5? Our default is to locate all datasets outside the yolov5 repo:

yolov5/data/coco128.yaml

Lines 1 to 8 in 37acbdc

    
           # COCO 2017 dataset http://cocodataset.org - first 128 training images 
        
           # Download command:  python -c "from yolov5.utils.google_utils import *; gdrive_download('1n_oKgR81BJtqk75b00eAjdv03qVCQn2f', 'coco128.zip')" 
        
           # Train command: python train.py --data coco128.yaml 
        
           # Default dataset location is next to /yolov5: 
        
           #   /parent_folder 
        
           #     /coco128 
        
           #     /yolov5

Borda · 2020-07-16T17:09:24Z

ok, no problem with the path change, but be aware that test.py is associated with pytest/unittests and has its specific meaning so having it for evaluation is very confusing...

BTW, you are allowed to edit this PR :]

glenn-jocher · 2020-07-16T18:03:49Z

@Borda ok thanks!

glenn-jocher · 2020-07-16T18:06:41Z

Do you know if there is a simple way to revert changes to a file using the PR web browser without having to revert it line by line?

I think the CI addition looks great, and very useful, I'd like to pull this file by itself at the moment.

glenn-jocher · 2020-07-16T18:34:58Z

I see all of the checks are passing now, very nice! Was reviewing the docs:
https://docs.github.com/en/actions/reference/virtual-environments-for-github-hosted-runners

Would it make sense to update the os to:
os: [ubuntu-latest]

Would it be possible to run the same tests in all 3 os?
os: [ubuntu-latest, macos-latest, windows-latest]

glenn-jocher · 2020-07-16T20:20:32Z

That was failing for missing numpy even it is part of the requirements.txt (even there was fixed version)

Hmm ok. Yes the dependencies can be tricky sometimes. Ok well can you close this PR, and submit a new one with just the check .yml file? I'll see if I can try to update it to work with the existing requirements.txt file, because otherwise it may break the cocoapi functionality in the colab notebook that is working now.

Borda · 2020-07-16T20:26:29Z

There is no need to, just to edit this one...

glenn-jocher · 2020-07-16T20:38:29Z

I just checked the numpy-cocoapi problem, it seems to be resolved now when I run the colab notebook (with numpy 1.18.5). So I've updated requirements.txt now to this, and included your torchvision constraint as well.

Borda · 2020-07-16T22:45:12Z

@glenn-jocher I have changed the evaluation method, but pls lets keep the eval file name as the test is too confusing...

glenn-jocher · 2020-07-16T23:09:52Z

@Borda I understand. I really appreciate your contributions here, but we want to limit the scope of this PR, for example to what the title says, adding GH action cpu tests only. I think your added *.yml does a great job of this. Updating 8 more files throughout the repository and renaming core files that will break existing workflows for many people is far beyond the scope of what we are looking for.

Borda · 2020-07-16T23:17:35Z

@glenn-jocher reverted renaming...

glenn-jocher · 2020-07-17T00:27:43Z

Yay thank you!!

glenn-jocher · 2020-07-17T00:29:52Z

You are right that the naming may be confusing, but such a large change is something we probably want to introduce in a major release (i.e. v1.0 to v2.0).

There are two functions right now that do inference, detect.py and test.py. It may very well make sense to try to unify them in the future as an eval.py.

Borda · 2020-07-17T06:23:28Z

Talking about releases, in such case I would recommend to make it as single package and make it installable so you can nicely put it on PyPI =)
If you are interested I can send such PR for you...

glenn-jocher · 2020-07-17T06:44:47Z

@Borda yes that might be a nice addition! The closest we have to that right now is loading yolov5 through pytorch hub.

I'm aiming to issue a new release in about 2 weeks, once I finish training some slightly updated models. I'll reach out to you then to see if you can help us put together a pip package!

Borda · 2020-07-17T07:09:57Z

Sure, it shall not be too hard wor, a few hours I guess :]
Jut noticed that you are merging PR with all commits I would highly recommend using Squash-merge which merge each PR as single commit to master so it gives you more control in any checking history or occasionally reverting to past state... also with this blaming commits from multiple branches/PRs running in parallel, it is super hard to trace meaningful changes...
To do you can simply check that the only possible merge strategy for PR is "Squash and merge" ;)

glenn-jocher · 2020-07-17T07:18:04Z

@Borda yes, you're absolutely right. I try to remember to squash and merge every time but since it's not the default I find myself merging with all commits sometimes by accident. Are you saying you can change the merge options in the settings?

glenn-jocher · 2020-07-17T07:20:05Z

@Borda ah yes, just found this in the settings and updated it to only allow squash and merge now. Great, thanks for the help, one small improvement at a time!

glenn-jocher · 2020-07-21T04:20:41Z

@Borda I wanted to give you an update on v2.0. Most of the parts are in place, I'm waiting for a new yolov5x to finish training. It's at 190/300 epochs, training about 1 epoch/hr means it will be done roughly on Friday. After this I should be all set to merge a PR with the updated models and code changes, and then to write up a 2.0 release shortly afterwards.

What would be the steps then for compiling pip packages for the 2.0 version? Do we need to register the repo or a user with the pip people to have them host our packages? Do we create 3 separate packages for the 3 main OS's? I've updated your CI code by the way to run tests on all 3 OS's and they are all currently passing now, so we appear to currently enjoy a minimum level of error-free compatibility across operating systems.

Borda · 2020-07-21T07:17:51Z

@glenn-jocher I have created a draft #465 so let's continue this discussion there :]

glenn-jocher · 2020-07-30T20:41:43Z

@Borda I just realized we ideally want to add a time condition for running the CI tests (i.e. ideally run them every 24 hours) in addition to the action conditions on-push and on-pr.

For example even if there are no pushes and no PRs, changes in the operating systems or the dependencies may affect results, as we've seen with the pytorch 1.6 release.

Borda · 2020-07-30T21:02:41Z

yeah, you can add a crone which will trigger it from time to time...

glenn-jocher · 2020-07-30T21:08:31Z

Would we do it like this?

I see the stale greeting is on a cron:

on:
  schedule:
  - cron: "0 0 * * *"

Wheras the CI tests use a list:

on: [push, pull_request]

From https://docs.github.com/en/actions/reference/events-that-trigger-workflows#example-using-multiple-events-with-activity-types-or-configuration it looks like the combined version would be this?

on:
  push:
  pull_request:
  schedule:
  - cron: "0 0 * * *"

Borda · 2020-07-30T21:14:06Z

never tried this combination, but this may work...

on:
  push
  pull_request
  schedule:
    cron: "0 0 * * *"

glenn-jocher · 2020-07-30T21:16:23Z

Created #572 to try this out. It will be good experiment!

glenn-jocher · 2020-07-31T07:04:50Z

@Borda I think it's working! I see it ran on schedule today :)

Borda added 6 commits July 16, 2020 09:35

add GH action tests

94b2bb6

requirements

a0fa5f5

requirements

1a503f3

requirements

a79633a

fix tests

5b46305

add badge

613b9d6

Borda marked this pull request as ready for review July 16, 2020 08:34

Borda changed the title ~~add GH action tests~~ add GH action CPU tests Jul 16, 2020

Borda added 8 commits July 16, 2020 11:16

lower batch-size

3bf77ca

weights

f0d1e09

args

3e6868d

parallel

7d8e2dd

rename eval

df270ea

rename eval

9a880a4

paths

da6c7ce

rename

eac0dbc

Borda added 2 commits July 16, 2020 12:25

lower bs

e8b7b86

timeout

5c73caa

Borda added 3 commits July 16, 2020 13:57

less xOS

2b8fbda

drop xOS

eafe85b

git attrib

0128947

Borda added 2 commits July 16, 2020 19:10

paths

be55111

paths

4500b49

Update eval.py

1e831bd

glenn-jocher and others added 4 commits July 16, 2020 13:38

Merge branch 'master' into tests/add-gh-action

69d46fd

Update ci-testing.yml

4171df3

Update ci-testing.yml

670d4ff

rename test

ef6f5b3

revert test module to confuse users...

e8ea772

glenn-jocher merged commit 2efa01d into ultralytics:master Jul 17, 2020

Borda deleted the tests/add-gh-action branch July 17, 2020 06:21

Borda mentioned this pull request Jul 21, 2020

make installable pip package #465

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add GH action CPU tests #426

add GH action CPU tests #426

Borda commented Jul 16, 2020 •

edited by UltralyticsAssistant

Loading

Borda commented Jul 16, 2020

Borda commented Jul 16, 2020

glenn-jocher commented Jul 16, 2020

Borda commented Jul 16, 2020 •

edited

Loading

glenn-jocher commented Jul 16, 2020

glenn-jocher commented Jul 16, 2020

glenn-jocher commented Jul 16, 2020 •

edited

Loading

glenn-jocher commented Jul 16, 2020

Borda commented Jul 16, 2020

glenn-jocher commented Jul 16, 2020

Borda commented Jul 16, 2020

glenn-jocher commented Jul 16, 2020

Borda commented Jul 16, 2020

glenn-jocher commented Jul 17, 2020

glenn-jocher commented Jul 17, 2020

Borda commented Jul 17, 2020 •

edited

Loading

glenn-jocher commented Jul 17, 2020

Borda commented Jul 17, 2020

glenn-jocher commented Jul 17, 2020

glenn-jocher commented Jul 17, 2020 •

edited

Loading

glenn-jocher commented Jul 21, 2020 •

edited

Loading

Borda commented Jul 21, 2020

glenn-jocher commented Jul 30, 2020

Borda commented Jul 30, 2020

glenn-jocher commented Jul 30, 2020 •

edited

Loading

Borda commented Jul 30, 2020 •

edited

Loading

glenn-jocher commented Jul 30, 2020

glenn-jocher commented Jul 31, 2020

add GH action CPU tests #426

add GH action CPU tests #426

Conversation

Borda commented Jul 16, 2020 • edited by UltralyticsAssistant Loading

🛠️ PR Summary

🌟 Summary

📊 Key Changes

🎯 Purpose & Impact

Borda commented Jul 16, 2020

Borda commented Jul 16, 2020

glenn-jocher commented Jul 16, 2020

Borda commented Jul 16, 2020 • edited Loading

glenn-jocher commented Jul 16, 2020

glenn-jocher commented Jul 16, 2020

glenn-jocher commented Jul 16, 2020 • edited Loading

glenn-jocher commented Jul 16, 2020

Borda commented Jul 16, 2020

glenn-jocher commented Jul 16, 2020

Borda commented Jul 16, 2020

glenn-jocher commented Jul 16, 2020

Borda commented Jul 16, 2020

glenn-jocher commented Jul 17, 2020

glenn-jocher commented Jul 17, 2020

Borda commented Jul 17, 2020 • edited Loading

glenn-jocher commented Jul 17, 2020

Borda commented Jul 17, 2020

glenn-jocher commented Jul 17, 2020

glenn-jocher commented Jul 17, 2020 • edited Loading

glenn-jocher commented Jul 21, 2020 • edited Loading

Borda commented Jul 21, 2020

glenn-jocher commented Jul 30, 2020

Borda commented Jul 30, 2020

glenn-jocher commented Jul 30, 2020 • edited Loading

Borda commented Jul 30, 2020 • edited Loading

glenn-jocher commented Jul 30, 2020

glenn-jocher commented Jul 31, 2020

Borda commented Jul 16, 2020 •

edited by UltralyticsAssistant

Loading

Borda commented Jul 16, 2020 •

edited

Loading

glenn-jocher commented Jul 16, 2020 •

edited

Loading

Borda commented Jul 17, 2020 •

edited

Loading

glenn-jocher commented Jul 17, 2020 •

edited

Loading

glenn-jocher commented Jul 21, 2020 •

edited

Loading

glenn-jocher commented Jul 30, 2020 •

edited

Loading

Borda commented Jul 30, 2020 •

edited

Loading