-
Notifications
You must be signed in to change notification settings - Fork 41
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 Image Dataloader for PASCAL VOC and CIFAR 10 dataset. #13
Conversation
302375c
to
94fba51
Compare
Hi @kartikdutt18. Regarding Please let me know what you think |
Hey @KimSangYeon-DGU, This is something I missed while making the proposal, The images in PASCAL dataset have different sizes, I can preprocess the images in python and we can store that dataset or We can add a resize feature that uses Bilinear interpolation from mlpack to resize images. Kindly let me know what you think. |
@kartikdutt18 Ok, then, let's do that :) I don't mind if you do this work on either this PR or another. |
Great, I'll do it in this PR first, if get a bit large I can open another one with just that. Thanks a lot. |
Yes, the best is to implement and use resizing feature in mlpack :) BTW, please let me know if this work turns into a burden for the summer work. We can adjust by going another way as you said above comment. |
Thanks a lot, If things go well I should have the resize function ready by tomorrow or the day after. Also, I think it'll be nice to have that feature. |
Or we can use the third library for image processing like |
Right, That would be much easier. I think have the conversion code for opencv matrix to armadillo so that should be easy to do. Thanks a lot for the amazing suggestion. |
Yes, and as you know, if you need to use OpenCV, you can do that in |
ddb2d98
to
2727851
Compare
Hey @KimSangYeon-DGU, I completed the main function for PASCAL Dataloader i.e.
Kindly let me know what you think. |
Hey @KimSangYeon-DGU, just another update. I have tests for both the data loader as well as augmentation class. I have created a subset that we would can upload to mlpack.org that can be used for testing. Only one thing is left i.e. Calling function through the constructor. If this makes sense I can add that in the next commit. |
Nice @kartikdutt18! Since #12 got merged, can you re-base this branch? it helps to review. Meanwhile, I'll look through this PR :) |
015e197
to
c058367
Compare
augmentation/augmentation_impl.hpp
Outdated
// Not sure how to avoid a copy here. | ||
DatasetType output; | ||
resizeLayer.Forward(dataset, output); | ||
dataset = output; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have resolved all conflicts. The one thing I am unsure of is how to avoid a copy here. For very large datasets resizing will simply not be possible incase the program quits as they run out of RAM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about you use this?
// Not sure how to avoid a copy here. | |
DatasetType output; | |
resizeLayer.Forward(dataset, output); | |
dataset = output; | |
// Not sure how to avoid a copy here. | |
DatasetType output; | |
resizeLayer.Forward(dataset, output); | |
dataset = std::move(output); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh makes sense. Awesome, Thanks a lot for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more thing I was planning to change was to use fields in labels. So when labels are passed in preprocessor function for YOLO model it would easier than doing look ups. Currently each labels has a different image, we could use a field vector and store multiple bounding boxes in one column rather than having a separate column for each bounding box. And the build failure is associated to zip file, I'll switch to tar / tar.gz and it should be fixed. Kindly let me know if this makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently each labels has a different image, we could use a field vector and store multiple bounding boxes in one column rather than having a separate column for each bounding box
Can you do that as a PoC? and then, let's see which one is better. I need to figure out how the field vector works.
At first, I thought there will be a problem in using linear algebra operations when we put multiple bounding boxes into one column. However, as you said, each label has a different shape of image, so we could do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'll make the changes now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Repeat for classification of imagenet
(Since the code is similar it will only take a day or two after completing PASCAL VOC).
I think this PR is almost ready, I am working on the last to-do now. Will complete it by tomorrow so that we can have the same functionality as FlowFromDirectory (keras) for image datasets. The Pascal dataloader is complete. It takes roughly 18 or more minutes to load all the images though. About the field vector I'll create another branch same as this one and post this link here. If that is better I can simply replace the commits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added image dataloader with flow from directory functionality. We can now support voc-detection, voc-classification and cifar-10 / 100. I'll set up the constructor and do the clean up as well. The build error in linux / windows now is because they have .DS_Store file. I'll add a check for that as well. Currently just manually checking for the file. Regards.
Also one thing that we could add maybe later (or open an issue) is addition of script to convert csv to xml files. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some Style related comments :)
Style fixes Style fixes
e97e616
to
adade09
Compare
This PR is migrated to #18. Thanks a lot. |
Hey everyone,
This is is a WIP of image data loader for this repo.
TO DO :
(maybe another PR or in this one, kindly let me know).
(Since the code is similar it will only take a day or two after completing PASCAL VOC).