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

VGG Classification network #2083

Closed
wants to merge 10 commits into from
Closed

VGG Classification network #2083

wants to merge 10 commits into from

Conversation

@jeffin07
Copy link
Contributor

jeffin07 commented Jun 8, 2020

Describe your change:

Added a deep-learning method for image classification #1809 using pytorch

  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.
jeffin07 and others added 7 commits Jun 6, 2020
@cclauss
Copy link
Member

cclauss commented Jun 8, 2020

What is https://en.wikipedia.org/wiki/VGG and why does the reader need to guess?

@jeffin07
Copy link
Contributor Author

jeffin07 commented Jun 8, 2020

It's the implementation of https://arxiv.org/abs/1409.1556 a classification network

jeffin07 and others added 2 commits Jun 8, 2020
github-actions github-actions
requirements.txt Show resolved Hide resolved
@TravisBuddy
Copy link

TravisBuddy commented Jun 8, 2020

Hey @jeffin07,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: 7110f230-a9a7-11ea-9161-4f77e253115e
@TravisBuddy
Copy link

TravisBuddy commented Jun 8, 2020

Hey @jeffin07,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: 28f2ebb0-a9a8-11ea-9161-4f77e253115e
@cclauss
Copy link
Member

cclauss commented Jun 8, 2020

It's the implementation of https://arxiv.org/abs/1409.1556 a classification network

The problem is that the URL does not contain the acronym VGG and the code comments do not explain what VGG stands for. It is going to confuse the reader to have class without knowing what the class name stands for.

@TravisBuddy
Copy link

TravisBuddy commented Jun 8, 2020

Hey @jeffin07,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: 4b310660-a9aa-11ea-9161-4f77e253115e
return f"VGG for feature extraction : \n {self.net}"


cfg = {

This comment has been minimized.

Copy link
@deadshotsb

deadshotsb Jul 3, 2020

Member

This repository is based on learning purpose. So, it will be better if you could provide an explanation of the usage of cfg parameter.

@deadshotsb
Copy link
Member

deadshotsb commented Jul 3, 2020

@jeffin07 You can change the VGG to VGG-16/19 term as they are mainly the standard ones and people refer to them mostly.

@deadshotsb
Copy link
Member

deadshotsb commented Jul 3, 2020

@cclauss VGG is a CNN classification, you can get it by using VGG-16/19.

There is one problem though, the PR contains the model defination using pytorch and no insights to the layers(their functionality) but am not so sure that if you could call this as a work in algorithms.

@deadshotsb deadshotsb requested a review from cclauss Jul 3, 2020
@cclauss
Copy link
Member

cclauss commented Jul 3, 2020

  • What does the V stand for?
  • What does the first G stand for?
  • What does the second G stand for?
@deadshotsb
Copy link
Member

deadshotsb commented Jul 3, 2020

VGG(Visual Geometry Group) or also OxfordNet. Similarly there are AlexNet or ResNET, etc

@cclauss
Copy link
Member

cclauss commented Jul 3, 2020

Thanks, please change the filename to visual_geometry_group.py or oxford_net.py.

@deadshotsb
Copy link
Member

deadshotsb commented Jul 3, 2020

@cclauss Actually VGG is often used and using a VGG-16/19 model and renaming the file as vgg_16 / vgg_19 would be beneficial, like using kmp instead of Knuth Morris Pratt.

@cclauss
Copy link
Member

cclauss commented Jul 3, 2020

In this repo we call it Knuth Morris Pratt instead of kmp. We do that very deliberately because we are trying to teach the readers of our code. We avoid unexplained acronyms because we want our readers to understand the origins and nomenclature of these algorithms. The 16 and 19 are config choices that have proven to be effective but they are not the algorithm. https://www.robots.ox.ac.uk/~vgg/research/very_deep

@jeffin07 jeffin07 closed this Jul 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.