changes to eval cli : option to eval on entire dataset

[ArshdeepSekhon]

No description provided.

[jinyongyoo]

I'm also wondering if it's just possible to achieve the same behavior using --num-examples argument since that is always processed together with a dataset. Having both --num-examples and --test-on-full-dataset seems a bit redundant to me. Instead of --num-examples always being a int, we can maybe allow passing a string like all (or -1 as a Pythonic way to index last element) to specify that we want to attack all samples. Then, in the parse_dataset_from_args function in textattack/commands/attack/attack_args_helpers.py, we can set args.num_examples=len(dataset). That way, we support not only eval command, but also attack and we don't have to worry about keeping track of two arguments.

[jxmorris12]

I'm also wondering if it's just possible to achieve the same behavior using --num-examples argument since that is always processed together with a dataset. Having both --num-examples and --test-on-full-dataset seems a bit redundant to me. Instead of --num-examples always being a int, we can maybe allow passing a string like all (or -1 as a Pythonic way to index last element) to specify that we want to attack all samples. Then, in the parse_dataset_from_args function in textattack/commands/attack/attack_args_helpers.py, we can set args.num_examples=len(dataset). That way, we support not only eval command, but also attack and we don't have to worry about keeping track of two arguments.

I see what you mean Jeffrey! @ArshdeepSekhon can you add one more thing: a check to make sure both arguments aren't set? Like if someone calls `textattack eval --num-examples=50 --test-on-full-dataset we should throw an error bc we don't know which one to do.

Btw @jinyongyoo I think if we switched to click or another command-line package we'd have better support for this type of thing!

[qiyanjun]

@ArshdeepSekhon I will close this since you merge similar changes to the #324

[Opdoop]

In the end, how to eval on entire dataset?
I tried the mentioned three way at textattack 0.2.14

• --num-examples -1
• --num-examples all
• --test-on-full-dataset

Sadly, none of the above works.

[qiyanjun]

@Opdoop we put this on hold due to final exams.. will update in a week

[ArshdeepSekhon]

Added -1 for attack and eval on entire dataset, sets args.num_examples = len(dataset) if num-examples is specified by user as -1.

[qiyanjun]

@Opdoop please try now!

[Opdoop]

@qiyanjun Success with --num-examples -1
log:

textattack eval --model lstm-imdb --num-examples -1
textattack: train_args.json not found in model path models/classification/lstm/imdb. Defaulting to 2 labels.
textattack: Loading pre-trained TextAttack LSTM: lstm-imdb
Reusing dataset imdb (/home/nano/.cache/huggingface/datasets/imdb/plain_text/1.0.0/90099cb476936b753383ba2ae6ab2eae419b2e87f71cd5189cb9c8e5814d12a3)
textattack: Loading datasets dataset imdb, split test.
textattack: Got 25000 predictions.
textattack: Successes 20535/25000 (82.14%)

One thing I want to confirm, the Successes here means classification result on normal test/val set. Thus the accuracy of the available model lstm-imdb here is 82.14%. Is this correct?

[qiyanjun]

https://textattack.readthedocs.io/en/latest/3recipes/models.html#lstm

…

On Nov 28, 2020, at 07:45, Opdoop ***@***.***> wrote:  @qiyanjun Success with --num-examples -1 log: textattack eval --model lstm-imdb --num-examples -1 textattack: train_args.json not found in model path models/classification/lstm/imdb. Defaulting to 2 labels. textattack: Loading pre-trained TextAttack LSTM: lstm-imdb Reusing dataset imdb (/home/nano/.cache/huggingface/datasets/imdb/plain_text/1.0.0/90099cb476936b753383ba2ae6ab2eae419b2e87f71cd5189cb9c8e5814d12a3) textattack: Loading datasets dataset imdb, split test. textattack: Got 25000 predictions. textattack: Successes 20535/25000 (82.14%) One thing I want to confirm, the Successes here means classification result on normal test/val set. Thus the accuracy of the available model lstm-imdb here is 82.14%. Is this correct? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[jinyongyoo]

Is it possible to rebase this on the master branch and clean up the commits? Seems like we have way too many commits (169!) coming from master.

[jinyongyoo]

Nvm didn't see that this PR was already merged.

[qiyanjun]

@jinyongyoo i wondered that too.. @jinyongyoo @ArshdeepSekhon is it because this is way behind master branch?

[qiyanjun]

@jinyongyoo i merged it because the request functions are in. And all tests pass. Should we revert?

[Opdoop]

@qiyanjun Em... I have seen this doc before. Succesees and Accuracy looks confusing to me. So I just want to confirm these two means the same.

[ArshdeepSekhon]

@qiyanjun Yes this was behind, I rebased it to master

[qiyanjun]

Good call. Let me clean the mentions.

…

On Nov 28, 2020, at 08:18, Opdoop ***@***.***> wrote:  @qiyanjun Em... I have seen this doc before. Succesees and Accuracy looks confusing to me. So I just want to confirm these two means the same. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.