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 CNN-MLP Model #13

Merged
merged 6 commits into from Aug 2, 2017
Merged

Add CNN-MLP Model #13

merged 6 commits into from Aug 2, 2017

Conversation

mdda
Copy link
Contributor

@mdda mdda commented Jul 5, 2017

To have a clean A/B kind of model, this PR factors out the CNN input stage and the final FC stage. This highlights the differences between the two models clearly.

The specific CNN-MLP implementation has been taken from yangky11's fork. However, since my branch was taken after the '10x speedup' patch, it includes that too.

RN @ epoch 20 :  
   Test set: Relation accuracy: 87% | Non-relation accuracy: 99%
CNN-MLP  @ epoch 100 :  
   Test set: Relation accuracy: 66% | Non-relation accuracy: 69%

It's also now Python3-ready, and there are some other minor tweaks.

What do you think?

@kimhc6028
Copy link
Owner

I am very sorry for late reply. I have been on military training until now. I cannot check your pr now. But it seems your code works beautifully. Thank you for your code! Next reply should be in 3 weeks later :)

@kimhc6028 kimhc6028 merged commit 1703277 into kimhc6028:master Aug 2, 2017
@mdda
Copy link
Contributor Author

mdda commented Aug 2, 2017

Great - and thanks for coming up with the original code so quickly. I used it as the basis of my presentation at the first PyTorch event in Singapore, which wasn't videoed but my slides are here.

Stay safe!

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.

None yet

2 participants