Skip to content

Add YOLO class. - #24

Merged
KimSangYeon-DGU merged 8 commits into
mlpack:masterfrom
kartikdutt18:Yolov1
Aug 31, 2020
Merged

Add YOLO class.#24
KimSangYeon-DGU merged 8 commits into
mlpack:masterfrom
kartikdutt18:Yolov1

Conversation

@kartikdutt18

@kartikdutt18kartikdutt18 commented Jul 9, 2020•
edited
Loading

Copy link
Copy Markdown
Member

Hey everyone,
This PR aims to YOLO class to the models repo. The following is the TO-DO list:

  • Add PreProcessor Function (Convert DataLoader annotations to YOLO targets). (I'll add this by 10.07.20)

  • Add Detection stage for the YOLO class (Where loss is calculated, feature maps are converted to understandable format).

  • Add YOLO model architecture.

@kartikdutt18

kartikdutt18 commented Jul 9, 2020

Copy link
Copy Markdown
MemberAuthor

The build failure occurs because it doesn't have the newer batchnorm yet.

@zoq

zoq commented Jul 14, 2020

Copy link
Copy Markdown
Member

I guess with mlpack/mlpack#2474 merged, this should build now? Let's rerun the tests.

@kartikdutt18

kartikdutt18 commented Jul 15, 2020

Copy link
Copy Markdown
MemberAuthor

I guess with mlpack/mlpack#2474 merged, this should build now? Let's rerun the tests.

Yes, it builds now.

@saksham189

saksham189 commented Aug 13, 2020•
edited
Loading

Copy link
Copy Markdown
Member

Can you take a look at why the style build is failing?

@kartikdutt18

kartikdutt18 commented Aug 13, 2020

Copy link
Copy Markdown
MemberAuthor

Sure, I will resolve it with the next commit. Thanks for pointing it out.

@KimSangYeon-DGU

KimSangYeon-DGU commented Aug 22, 2020

Copy link
Copy Markdown
Member

Hey @kartikdutt18 Can you rebase this PR?

KimSangYeon-DGU
KimSangYeon-DGU reviewed Aug 22, 2020
View reviewed changes

@KimSangYeon-DGUKimSangYeon-DGU left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I left some comments

Comment threadmodels/yolo/yolo.hpp Outdated
Comment threadmodels/yolo/yolo.hpp Outdated
Comment threadmodels/yolo/yolo_impl.hpp Outdated
if (weights == "voc")
{
// Download weights here.
LoadModel("./../weights/YOLO/yolo" + YOLOVersion + "_voc.bin");

@KimSangYeon-DGUKimSangYeon-DGUAug 22, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please let us know if the weight file is ready. Do you intend to use the weight converter?

@kartikdutt18kartikdutt18Aug 22, 2020

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Yes I do intend to use the weights. I am done with the implementation of detector too, It makes some extra copies which can be avoided which I'll remove and then I'll leave a comment here when everything is ready.

@kartikdutt18kartikdutt18Aug 23, 2020

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I'm done with the detector part as well and I'm moving forward with testing the model. If everything works well, I'll add the YOLO model in converter repo for you to test it and then we can merge this PR.

@kartikdutt18kartikdutt18Aug 26, 2020

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I have added a simple tests in this PR. I haven't added tests for weights because of mlpack/mlpack#2561. I think we can merge this PR since the model is working and any changes would be internal implementation in mlpack. To see if the model is working correctly you can run the following commands :

git clone https://github.com/kartikdutt18/mlpack-PyTorch-Weight-Translator.git
cd mlpack-PyTorch-Weight-Translator
./run.sh yolov1_tiny

With this you will see the model weights being created and transferred into the YOLO model and being used for prediction. You can see that the iou of predictions made by PyTorch model and YOLO model are same i.e. 1.0. Kindly let me know if you face any issues. If this makes sense, I think we can merge this PR and then later add a PR with tests for weights once mlpack/mlpack#2561 is resolved.
(Similar comment from DarkNet).

@kartikdutt18kartikdutt18Aug 26, 2020

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

With this we should also merge #29 and #25 as well.

@KimSangYeon-DGUKimSangYeon-DGUAug 27, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@kartikdutt18 Rather than creating a weight file, how about save it to the weight directory for making it ready-for-use?

@kartikdutt18kartikdutt18Aug 27, 2020

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

This would face the same problem as DarkNet since this also uses BatchNorm Layer which has a serialization issue.

KimSangYeon-DGU
KimSangYeon-DGU approved these changes Aug 27, 2020
View reviewed changes

@KimSangYeon-DGUKimSangYeon-DGU left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No further comments from my side.

KimSangYeon-DGU
KimSangYeon-DGU reviewed Aug 27, 2020
View reviewed changes

@KimSangYeon-DGUKimSangYeon-DGU left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Left some comments about styles

Comment threadmodels/yolo/yolo.hpp Outdated
Comment threadmodels/yolo/yolo.hpp Outdated
Comment threadmodels/yolo/yolo.hpp Outdated
Comment threadmodels/yolo/yolo.hpp Outdated
Comment threadmodels/yolo/yolo.hpp Outdated
Comment threadmodels/yolo/yolo.hpp Outdated
Comment threadmodels/yolo/yolo.hpp Outdated
KimSangYeon-DGU
KimSangYeon-DGU requested changes Aug 27, 2020
View reviewed changes
Comment threadmodels/yolo/yolo_impl.hpp
Comment threadmodels/yolo/yolo_impl.hpp
KimSangYeon-DGU
KimSangYeon-DGU approved these changes Aug 27, 2020
View reviewed changes

@KimSangYeon-DGUKimSangYeon-DGU left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No further comments from my side.

mlpack-bot[bot]
mlpack-botBot approved these changes Aug 28, 2020
View reviewed changes

@mlpack-botmlpack-botBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Second approval provided automatically after 24 hours. 👍

@KimSangYeon-DGU
KimSangYeon-DGU merged commit 49dcafb into mlpack:masterAug 31, 2020
@KimSangYeon-DGU

KimSangYeon-DGU commented Aug 31, 2020

Copy link
Copy Markdown
Member

Thanks for another great contribution 👍

@kartikdutt18

kartikdutt18 commented Aug 31, 2020

Copy link
Copy Markdown
MemberAuthor

Awesome, Thanks a lot @KimSangYeon-DGU and @saksham189 for all the reviews, suggestions and the help.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@KimSangYeon-DGUKimSangYeon-DGUKimSangYeon-DGU approved these changes
@mlpack-botmlpack-bot[bot]mlpack-bot[bot] approved these changes

Assignees

No one assigned

Labels

t: added feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kartikdutt18@zoq@saksham189@KimSangYeon-DGU