Skip to content

Image Dataloader with Field type - #18

Merged
KimSangYeon-DGU merged 4 commits into
mlpack:masterfrom
kartikdutt18:ImageDataloadersBetter
Jun 11, 2020
Merged

Image Dataloader with Field type#18
KimSangYeon-DGU merged 4 commits into
mlpack:masterfrom
kartikdutt18:ImageDataloadersBetter

Conversation

@kartikdutt18

@kartikdutt18kartikdutt18 commented Jun 5, 2020•
edited
Loading

Copy link
Copy Markdown
Member

This is nearly the same PR as the other image dataloader. Difference is that I have used field vector inserting bounding boxes.
So in preprocessing of dataset for model we would have instant lookups.

kartikdutt18
kartikdutt18 commented Jun 5, 2020
View reviewed changes
Comment threaddataloader/dataloader.hpp Outdated
@zoq

zoq commented Jun 5, 2020

Copy link
Copy Markdown
Member

This is nearly the same PR as the other image dataloader. Difference lies that I have used field vector inserting bounding boxes.
So in preprocessing of dataset for model we would have instant lookups.

So I guess we can close the other one?

@KimSangYeon-DGU

KimSangYeon-DGU commented Jun 5, 2020•
edited
Loading

Copy link
Copy Markdown
Member

@zoq Yes, we can close the other one if this is better :) We're currently seeing if which approach is better between this and #13.

@KimSangYeon-DGUKimSangYeon-DGU mentioned this pull request Jun 5, 2020
Closed
6 tasks
KimSangYeon-DGU
KimSangYeon-DGU reviewed Jun 5, 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.

Left some style and possible improvement comments, it looks good to go :)

Comment threadaugmentation/augmentation.hpp Outdated
Comment threaddataloader/dataloader.hpp Outdated
Comment threaddataloader/dataloader.hpp Outdated
Comment threaddataloader/dataloader.hpp Outdated
Comment threaddataloader/dataloader_impl.hpp
Comment threaddataloader/dataloader.hpp Outdated
Comment threaddataloader/dataloader.hpp Outdated
Comment threaddataloader/dataloader.hpp Outdated
Comment threaddataloader/dataloader_impl.hpp
Comment threadutils/utils.hpp Outdated
KimSangYeon-DGU
KimSangYeon-DGU reviewed Jun 5, 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.

Another comment

Comment threaddataloader/dataloader.hpp Outdated
saksham189
saksham189 reviewed Jun 7, 2020
View reviewed changes

@saksham189saksham189 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.

Just some minor comments. Looks great!

Comment threadaugmentation/augmentation.hpp Outdated
Comment threaddataloader/dataloader.hpp Outdated
@kartikdutt18
kartikdutt18force-pushed the ImageDataloadersBetter branch from 5b8e0c9 to 1218f53CompareJune 8, 2020 08:24
KimSangYeon-DGU
KimSangYeon-DGU reviewed Jun 8, 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 think it's almost ready to be merged, please check the comments including the source path of serverName in the CurlDownloadTest. I mean this.

Comment threadaugmentation/augmentation.hpp Outdated
Comment threadaugmentation/augmentation.hpp Outdated
Comment threadaugmentation/augmentation.hpp
Comment threadaugmentation/augmentation.hpp Outdated
Comment threaddataloader/dataloader.hpp
Comment threadaugmentation/augmentation.hpp Outdated
Comment threadaugmentation/augmentation.hpp Outdated
KimSangYeon-DGU
KimSangYeon-DGU reviewed Jun 8, 2020
View reviewed changes
Comment threaddataloader/datasets.hpp Outdated
saksham189
saksham189 reviewed Jun 8, 2020
View reviewed changes
Comment threadaugmentation/augmentation.hpp
saksham189
saksham189 reviewed Jun 8, 2020
View reviewed changes
Comment threadaugmentation/augmentation_impl.hpp Outdated
Comment threadaugmentation/augmentation_impl.hpp Outdated
zoq
zoq reviewed Jun 9, 2020
View reviewed changes

@zoqzoq 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.

Really nice to see this come to live, do you think we could provide a simple notebook to show how this code can be used to augment some image dataset?

Comment thread.travis.yml Outdated
Comment threadaugmentation/augmentation.hpp Outdated
Comment threadaugmentation/augmentation_impl.hpp
Comment threaddataloader/dataloader.hpp Outdated
Comment threaddataloader/dataloader.hpp Outdated
Comment threaddataloader/dataloader_impl.hpp
@kartikdutt18

kartikdutt18 commented Jun 10, 2020•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

do you think we could provide a simple notebook to show how this code can be used to augment some image dataset?

Sure I can give it a try, This would be for examples repo or model repo? Thanks.

KimSangYeon-DGU
KimSangYeon-DGU reviewed Jun 11, 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.

Looks great! I left some minor comments :)

Comment threadtests/dataloader_tests.cpp Outdated
Comment threadtests/dataloader_tests.cpp Outdated
Comment threadtests/dataloader_tests.cpp Outdated
Comment threadtests/dataloader_tests.cpp Outdated
Comment threadtests/dataloader_tests.cpp Outdated
Comment threadtests/dataloader_tests.cpp Outdated
@kartikdutt18
kartikdutt18force-pushed the ImageDataloadersBetter branch from b0a8c2c to 022ba45CompareJune 11, 2020 06:00
@KimSangYeon-DGU

KimSangYeon-DGU commented Jun 11, 2020

Copy link
Copy Markdown
Member

@kartikdutt18 I think this is ready to be merged. Can you squash the commits to be concise?

@kartikdutt18

kartikdutt18 commented Jun 11, 2020

Copy link
Copy Markdown
MemberAuthor

Sure. Let me do that right now.

@kartikdutt18
Add basic definition of models, Needs to be trained and tested
785905a
Trained a lenet1 model
Add All Weights
Add unzip utility function, fix linux build
Completed everything
Fix build, Style fixes next
Use force local for windows while unzipping tar files
Use force local for windows while unzipping tar files
Added Utility Function
Added Utility Function
Style Fix
Fix Typo causing build error in windows
Fix Typo causing build error in windows
Fix const issue for windows
Extract in data folder
Reposition force local
This should work
Print Path in windows for debugging
Print Path in windows for debugging
Print Path in windows for debugging
Stip components of tar
Stip components of tar
strip componenets
Initial Definition
Add List Dir utility function
Add List Dir utility function
Squash this, annotation read
Yay, we can read xml now
Complete labels portion of dataloader
Style Fix
Style Fix
Style Fix
Add basic definition of augmentation class
Load images according to annotation in column format, Reduce complexity by preventing whole iteration
Augmentation work
Augmentation work
@kartikdutt18
Complete LoadObjectDetection Function
6b306aa
Style fixes
Style fixes
@kartikdutt18
Add tests for augmentation
a975740
@kartikdutt18
kartikdutt18force-pushed the ImageDataloadersBetter branch from 022ba45 to c859e8eCompareJune 11, 2020 06:31
KimSangYeon-DGU
KimSangYeon-DGU approved these changes Jun 11, 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.

Amazing! Looks good to me 👍

@kartikdutt18

kartikdutt18 commented Jun 11, 2020

Copy link
Copy Markdown
MemberAuthor

Pushing a commit to delete the dataset.

@kartikdutt18
Add tests for dataloader
3fc6e7b
Upload dataset temp
Resolve merge conflicts
Resolve merge conflicts
Resolve merge conflicts
Resolve merge conflicts
Style fixes
Set up constructor
Complete Dataloader, write function definition for image dataloader
Build Fixed, Style Fixed
style fixed
Fix linux and Windows build by ignoring invalid files like .DS_Store
Add image dataloader, needs clean up
Style fixes
Check for file type
Check for correct extention and filetype
Style Fixed
Build Fixed, Nice way to avoid hidden files
Style Fixed
Clean Up
Why is config there even though I did a rebase
Add data split (internal), need implementation only for field type
Style Fixed
need to seperate the declaration
Fixed
style fixes, commits also need clean up
Remove template from Augmentation
Fixed the duplicacy error
Add mat type support (there is invalid read)
Mat type gives invalid read
Hmm,
style fixes, commits also need clean up
Typo -> invalid read -> Fixed
Remove extra lines
Rename cell to image
Style Fixes and use mlpack URL
Make augmentation case insensitive
Allow multiple Resize Transform
Add unknown augmentation warning
Style Fixes and boundary checks
Increase dataset size, adjust comments, change URL
Style Fixes and boundary checks
Remove locally held datasets
@kartikdutt18
kartikdutt18force-pushed the ImageDataloadersBetter branch from c859e8e to 3fc6e7bCompareJune 11, 2020 12:29
saksham189
saksham189 approved these changes Jun 11, 2020
View reviewed changes

@saksham189saksham189 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.

Thanks for the work on this! LGTM

@zoq

zoq commented Jun 11, 2020

Copy link
Copy Markdown
Member

do you think we could provide a simple notebook to show how this code can be used to augment some image dataset?

Sure I can give it a try, This would be for examples repo or model repo? Thanks.

I think it makes sense to put it into the examples repository, super excited to see this merged.

@KimSangYeon-DGU
KimSangYeon-DGU merged commit 5ea20fc into mlpack:masterJun 11, 2020
@KimSangYeon-DGU

KimSangYeon-DGU commented Jun 11, 2020

Copy link
Copy Markdown
Member

@kartikdutt18 Thanks for the amazing work!

@kartikdutt18

kartikdutt18 commented Jun 12, 2020

Copy link
Copy Markdown
MemberAuthor

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

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

Reviewers

@zoqzoqzoq left review comments
@saksham189saksham189saksham189 approved these changes
@KimSangYeon-DGUKimSangYeon-DGUKimSangYeon-DGU 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@KimSangYeon-DGU@saksham189