TalwalkarLab / leaf

Leaf: A Benchmark for Federated Settings
BSD 2-Clause "Simplified" License
852 stars 244 forks source link

Testing data and train data are repeated in Shakespeare #19

Closed zliangak closed 4 years ago

zliangak commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

zliangak commented 4 years ago

Yes, the issue remains.

scaldas commented 4 years ago

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 commented 4 years ago

Cool! Thanks a lot!