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

Add YOLO object detection model #552

Merged
merged 86 commits into from
Sep 10, 2021

Conversation

senarvi
Copy link
Contributor

@senarvi senarvi commented Feb 2, 2021

What does this PR do?

This PR adds the YOLO object detection model. The implementation is based on the YOLOv3 and YOLOv4 Darknet implementations, although it doesn't include all the features of YOLOv4. Detection seems to work with weights that have been trained using the Darknet implementation, so the network architecture should be more or less identical. The network architecture is read from a configuration file in the same format as in the Darknet implementation. It supports loading weights from a Darknet model file too, if you don't want to start training from a randomly initialized model.

Fixes #22

Before submitting

  • Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure your PR does only one thing, instead of bundling different changes together?
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests? [not needed for typos/docs]
  • Did you verify new and existing tests pass locally with your changes?
  • If you made a notable change (that affects users), did you update the CHANGELOG?

PR review

  • Is this pull request ready for review? (if not, please submit in draft mode)

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

@pep8speaks
Copy link

pep8speaks commented Feb 2, 2021

Hello @senarvi! Thanks for updating this PR.

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-08-11 15:29:11 UTC

@github-actions github-actions bot added datamodule Anything related to datamodules documentation Improvements or additions to documentation model labels Feb 2, 2021
@codecov
Copy link

codecov bot commented Feb 2, 2021

Codecov Report

Merging #552 (66c8111) into master (47eb2aa) will decrease coverage by 48.52%.
The diff coverage is 14.21%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #552       +/-   ##
===========================================
- Coverage   72.17%   23.64%   -48.53%     
===========================================
  Files         121      124        +3     
  Lines        7550     8098      +548     
===========================================
- Hits         5449     1915     -3534     
- Misses       2101     6183     +4082     
Flag Coverage Δ
cpu 23.64% <14.21%> (-48.53%) ⬇️
pytest 23.64% <14.21%> (-48.53%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pl_bolts/models/detection/yolo/yolo_module.py 12.80% <12.80%> (ø)
pl_bolts/models/detection/yolo/yolo_config.py 14.40% <14.40%> (ø)
pl_bolts/models/detection/yolo/yolo_layers.py 14.47% <14.47%> (ø)
pl_bolts/models/detection/__init__.py 100.00% <100.00%> (ø)
pl_bolts/models/rl/__init__.py 0.00% <0.00%> (-100.00%) ⬇️
pl_bolts/models/rl/common/agents.py 0.00% <0.00%> (-100.00%) ⬇️
pl_bolts/models/rl/dueling_dqn_model.py 0.00% <0.00%> (-100.00%) ⬇️
pl_bolts/models/rl/advantage_actor_critic_model.py 0.00% <0.00%> (-97.71%) ⬇️
pl_bolts/models/rl/double_dqn_model.py 0.00% <0.00%> (-95.84%) ⬇️
...l_bolts/models/rl/vanilla_policy_gradient_model.py 0.00% <0.00%> (-93.45%) ⬇️
... and 73 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 47eb2aa...66c8111. Read the comment docs.

Copy link
Contributor

@akihironitta akihironitta left a comment

Choose a reason for hiding this comment

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

We recently fixed some of the tests, so would you mind merging master branch? Also, it seems the change here is causing errors in CI, so would you mind resolving that, too?

pl_bolts/datamodules/vocdetection_datamodule.py Outdated Show resolved Hide resolved
@akihironitta akihironitta removed the documentation Improvements or additions to documentation label Feb 3, 2021
@senarvi senarvi mentioned this pull request Feb 9, 2021
@williamFalcon
Copy link
Contributor

williamFalcon commented Feb 12, 2021

this looks really good @senarvi!
@Borda @kaushikb11 let's get this merged?

* IoU loss functions take image space coordinates as input.
@senarvi
Copy link
Contributor Author

senarvi commented Aug 19, 2021

@Borda I noticed also the formatting issues and fixed them by running the new formatting tools.

@Borda
Copy link
Member

Borda commented Aug 25, 2021

@Borda I noticed also the formatting issues and fixed them by running the new formatting tools.

cool, just restarting CI

@senarvi
Copy link
Contributor Author

senarvi commented Aug 30, 2021

VOCDetectionDataModule was updated to better conform to the new data module interface. The constructor takes batch size and the transforms take image and target. We can now pass the data module to Trainer, instead of the dataloaders. train_dataloader() and val_dataloader() still support image-only transforms, but I don't see any need for using them.

@senarvi
Copy link
Contributor Author

senarvi commented Aug 30, 2021

The test suite found still one problem with integer division with an older PyTorch version, but it should be fixed now. @Borda could you restart the CI? I don't see the error when running the tests with my PyTorch installation.

@akihironitta
Copy link
Contributor

@senarvi Hi, thanks again for all your work and patience! Just restarted CI again. I'll try to be active on this PR for the next few days until we get this PR merged :]

@senarvi
Copy link
Contributor Author

senarvi commented Sep 9, 2021

@akihironitta correct me if I'm wrong, but the remaining error was something about CIFAR dataset and not related to this pull request.

Copy link
Contributor

@akihironitta akihironitta left a comment

Choose a reason for hiding this comment

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

@akihironitta correct me if I'm wrong, but the remaining error was something about CIFAR dataset and not related to this pull request.

@senarvi You're right that all of the related tests here have passed. Once the following comments gets resolved, I believe we can land this PR finally!

CHANGELOG.md Outdated Show resolved Hide resolved
pl_bolts/models/detection/__init__.py Outdated Show resolved Hide resolved
@senarvi
Copy link
Contributor Author

senarvi commented Sep 10, 2021

@akihironitta ok, done.

@Borda Borda merged commit b9b35c4 into Lightning-Universe:master Sep 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
datamodule Anything related to datamodules enhancement New feature or request model
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add yolo v3/4
10 participants