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 untrained alexnet #1

Merged
merged 3 commits into from
Jun 16, 2020
Merged

add untrained alexnet #1

merged 3 commits into from
Jun 16, 2020

Conversation

dnabanita7
Copy link

@dnabanita7 dnabanita7 commented Jun 13, 2020

I have added untrained AlexNet along with a master file for co-ordinating all the vision models. It is linked to issue.

Copy link

@darsnack darsnack left a comment

Choose a reason for hiding this comment

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

Good job! There are a couple changes I suggested to make the code more "Julian." For future models, you should use Metalhead as a reference to avoid replicating code, but you don't need to use their style of wrapping layers in a struct.

src/VisionModel/AlexNet.jl Outdated Show resolved Hide resolved
src/VisionModel/AlexNet.jl Outdated Show resolved Hide resolved
src/VisionModel/AlexNet.jl Outdated Show resolved Hide resolved
src/VisionModel/AlexNet.jl Outdated Show resolved Hide resolved
src/VisionModel/VisionModel.jl Outdated Show resolved Hide resolved
@dnabanita7
Copy link
Author

I have made all the requested changes.

Copy link

@darsnack darsnack left a comment

Choose a reason for hiding this comment

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

Perfect! Just some quick stylistic changes.

src/VisionModel/AlexNet.jl Outdated Show resolved Hide resolved
src/VisionModel/AlexNet.jl Outdated Show resolved Hide resolved
src/VisionModel/VisionModel.jl Outdated Show resolved Hide resolved
@darsnack
Copy link

Awesome!

@dnabanita7
Copy link
Author

Can you please merge it so that I will delete this branch and start working for resnet?

@darsnack
Copy link

darsnack commented Jun 16, 2020

I can't merge changes into MLH-Fellowship/FluxModels.jl. I don't know how your mentors want you to do stuff, but eventually you need to make a PR to darsnack/FluxModels.jl with these commits which I will merge.

Probably a good idea to sync up with your mentor and figure out exactly what the process they want you to follow is.


EDIT: Actually, looking through the Discord, you are supposed to fork darsnack/FluxModels.jl to MLH-Fellowship/FluxModels.jl. You make commits to MLH-Fellowship/FluxModels.jl directly (in some dev branch) then make PRs to darsnack/FluxModels.jl from MLH-Fellowship/FluxModels.jl

@darsnack
Copy link

I would get the MLH staff to give you write access to this repo. Then merge these changes into MLH-Fellowship. Create a new PR to darsnack/FluxModels.jl from MLH-Fellowship/FluxModels.jl. In the future, you should make commits to a dev branch of MLH-Fellowship/FluxModels.jl directly.

@dnabanita7
Copy link
Author

I have merge access now. So I am merging and creating the PR. I will follow the guidelines next time as I wasn't clear.

@dnabanita7 dnabanita7 merged commit 816702a into MLH-Fellowship:master Jun 16, 2020
@dnabanita7 dnabanita7 deleted the vision branch June 17, 2020 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants