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

Testing data and train data are repeated in Shakespeare #19

Closed
zliangak opened this issue Nov 30, 2019 · 7 comments
Closed

Testing data and train data are repeated in Shakespeare #19

zliangak opened this issue Nov 30, 2019 · 7 comments

Comments

@zliangak
Copy link

Using the splitting method provided in the paper_experiment, I found that the testing data appeals exactly in the training data, resulting in a 100% testing accuracy if you use SGD+2 layers LSTM to train it.

For example, in the training set, 'THE_FIRST_PART_OF_HENRY_THE_SIXTH_MORTIMER''s words are:
[..., g age, Let dying Mortimer here rest himself. Even like a man new haled from the ',
' age, Let dying Mortimer here rest himself. Even like a man new haled from the r',
'age, Let dying Mortimer here rest himself. Even like a man new haled from the ra',
'ge, Let dying Mortimer here rest himself. Even like a man new haled from the rac',
'e, Let dying Mortimer here rest himself. Even like a man new haled from the rack',
', Let dying Mortimer here rest himself. Even like a man new haled from the rack,',
'Let dying Mortimer here rest himself. Even like a man new haled from the rack, S',
'et dying Mortimer here rest himself. Even like a man new haled from the rack, So',
't dying Mortimer here rest himself. Even like a man new haled from the rack, So ',
' dying Mortimer here rest himself. Even like a man new haled from the rack, So f',
'dying Mortimer here rest himself. Even like a man new haled from the rack, So fa',
'ying Mortimer here rest himself. Even like a man new haled from the rack, So far',
'ing Mortimer here rest himself. Even like a man new haled from the rack, So fare',
'ng Mortimer here rest himself. Even like a man new haled from the rack, So fare ',
'g Mortimer here rest himself. Even like a man new haled from the rack, So fare m',...]

and in the testing set, you can find the exact santence:

[' Let dying Mortimer here rest himself. Even like a man new haled from the rack, ']

My model will get a testing accuracy about 1 in about 35 epochs.

@scaldas
Copy link
Collaborator

scaldas commented Dec 2, 2019

You are right. The split shouldn't be at random but temporal, and we should be careful to avoid this. We will work on fixing this.

Can you provide us with the parameters that you used to obtain the testing accuracy (learning rate, size of the layers, etc.)?

Thank you.

@zliangak
Copy link
Author

zliangak commented Dec 2, 2019

I am using pytorch. All info is shown in the file below. One different of my model is that I feed all the hidden unit (instead of the last one) in to the linear layer. When I use your implementation of LSTM, i.e. only feeding the last hidden unit, I can still get a pretty high accuracy given enough training epochs.

Hope this can be fixed soon. It is a very helpful dataset. Thanks.

test.pdf

@zliangak
Copy link
Author

zliangak commented Dec 4, 2019

Sorry, there is a mistake of my implementation in the cell-6 of "test.pdf". When I define test_loader, I should use dataset=test_set instead of dataset=train_set.

And this would not affect the existence of the problem regarding this issue.

Regards,

@scaldas
Copy link
Collaborator

scaldas commented Dec 5, 2019

Sorry, I'm confused by your update. Does this mean the issue remains?

@zliangak
Copy link
Author

zliangak commented Dec 6, 2019

Yes, the issue remains.

@scaldas
Copy link
Collaborator

scaldas commented Mar 11, 2020

I have modified the train/test splits for Shakespeare. They are now temporally split, and samples that would leak any test information into the training set are ignored. This means that, if the last training sample happens at index i, the first test sample happens at index i + seq_len. We use seq_len as 80.

A side effect of this change is that some users now don't have any test samples, and have to be dropped from training.

@zliangak
Copy link
Author

Cool! Thanks a lot!

mehreentahir16 pushed a commit to mehreentahir16/leaf that referenced this issue Mar 8, 2024
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

No branches or pull requests

2 participants