-
-
Notifications
You must be signed in to change notification settings - Fork 16.1k
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
add GH action CPU tests #426
Conversation
@glenn-jocher @dlawrences mind checking this one... |
maybe just one Python version for xOS as there are only 5 concurrency jobs in free version |
@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: Lines 1 to 8 in 37acbdc
|
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 :] |
@Borda ok thanks! |
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. |
I see all of the checks are passing now, very nice! Was reviewing the docs: Would it make sense to update the os to: Would it be possible to run the same tests in all 3 os? |
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. |
There is no need to, just to edit this one... |
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. |
@glenn-jocher I have changed the evaluation method, but pls lets keep the eval file name as the test is too confusing... |
@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. |
@glenn-jocher reverted renaming... |
Yay thank you!! |
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. |
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 =) |
@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! |
Sure, it shall not be too hard wor, a few hours I guess :] |
@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? |
@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! |
@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. |
@glenn-jocher I have created a draft #465 so let's continue this discussion there :] |
@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. |
yeah, you can add a crone which will trigger it from time to time... |
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 * * *" |
never tried this combination, but this may work... on:
push
pull_request
schedule:
cron: "0 0 * * *" |
Created #572 to try this out. It will be good experiment! |
@Borda I think it's working! I see it ran on schedule today :) |
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.ci-testing.yml
) for Continuous Integration (CI) testing on CPUs.weights/download_weights.sh
script for downloading weight files with an updated syntax.π― Purpose & Impact
These changes collectively aim to improve the project's code quality control, setup experience, and overall reliability for users and contributors.