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

Split Yolov5 in GPU/CPU versions #200

Open
2 tasks
jacobowitz opened this issue Sep 15, 2021 · 6 comments
Open
2 tasks

Split Yolov5 in GPU/CPU versions #200

jacobowitz opened this issue Sep 15, 2021 · 6 comments
Labels
good-first-issue Tasks suitable for first time contributors

Comments

@jacobowitz
Copy link
Contributor

jacobowitz commented Sep 15, 2021

While working on GPU support for a couple of executors in this issue, I've stumbled over a problem for the Yolov5 segmenter, which can be seem in this draft pr.
CI always fails to install pycocotools on our custom GPU github action runners.
That is fairly annoying, because we dont even need this dependency, it is only introduced because the original upstream authors of yolov5 dont provide a Pypi package and we are using this fork instead. There also have been some related discussions already: here and here

So this issue contains two sub tasks:

  • Ideally submit a PR to the original yolov5 repo and provide a setup.py
  • Replace the yolov5 fork with the real one and do necessary changes for separing gpu/cpu version of the yolov5 segmenter
@jacobowitz jacobowitz added the good-first-issue Tasks suitable for first time contributors label Sep 15, 2021
@hmen97
Copy link

hmen97 commented Sep 20, 2021

Hi @jacobowitz it looks like this pull request has added all the changes required for a setup.py but the authors are holding off on integrating it as it would involve adding a yolov5 base directory, so they are looking for other options. Meanwhile the author has mentioned this 3rd party Pypi package as the best way to do inference right now which is the same as the fork you referenced.
Maybe we can add the real implementation as a submodule?

@jacobowitz
Copy link
Contributor Author

@hmen97 thank you for digging into this issue! Its a pity that yolov5 authors won't refactor their repo to enable a proper Pypi package :(
I think adding their repo as a submodule is not an option as it would break a lot of automation and also does not fit in this general purpose repository here. I think another option might be to fork the fork we are using and change the requirements.txt to a more minimal one. Maybe a better option would be a PR against the existing fork to structure the requirements.txt a bit better? I think the unnecessary dependencies should be marked as such there.

@hmen97
Copy link

hmen97 commented Sep 20, 2021

Yeah I was thinking along the same lines, based on what you have said

CI always fails to install pycocotools on our custom GPU github action runners.

If I make a PR against the forked repo so that unecessary dependencies like pycocotools become optional/can be skipped when running setup.py, I think that should solve the issue, but in case they don't accept that change then I'll fork the fork. Is that alright?

@jacobowitz
Copy link
Contributor Author

@hmen97 that sounds great!

@CatStark
Copy link
Member

Hi @hmen97 did you have any chance to look into this? it this ticket still relevant to you? :)

@cristianmtr
Copy link
Contributor

Hey @hmen97

We've unassigned you from the issue for now, and moved the issue back to our backlog. If you want to continue your work on it, let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good-first-issue Tasks suitable for first time contributors
Projects
None yet
Development

No branches or pull requests

4 participants