Feature Request: Port the significance test?

[felix-schneider]

In moses, there is this script for determining the "true" BLEU score within a confidence interval. Unfortunately, it does not have the configurability that sacreBLEU has.

In order to compare systems with regard to statistical significance, it would be nice to have a similar script, but supporting sacreBLEU.

[martinpopel]

See #11 and https://github.com/Gldkslfmsd/MTCE-py/blob/master/mtce/bootstrap.py

[mjpost]

Yes, I'd love to have this in sacrebleu. It's just looking for a contributor...

[martinpopel]

The paired property is the main obstacle when adding paired bootstrap to SacreBLEU - we would need to change the interface, so that you can provide multiple systems' translations at once. SacreBLEU currently expects the system translation on the stdin and does not provide a way to name the system. We would need to change this e.g. to accepting tab-separated systems on the input and optionally providing system names, e.g. paste moses.txt nematus.txt | sacrebleu --bootstrap_samples=10000 --system_names=Moses,Nematus reference.txt.

[ozancaglayan]

maybe the significance part can be a wrapper command around sacrebleu?

[martinpopel]

Efficient implementation of bootstrap does not call sacrebleu 10000 times (doing the tokenization 10000 times), so a "wrapper command around sacrebleu" is not a proper solution. We could provide sacrebleu-bootstrap command, which would take multiple systems' translations on the input (and optionally their names as --system-names) and the number of resamples as a parameter, but I don't see any advantage over keeping this within sacrebleu.

[cfedermann]

I had implemented this way back but the feature was lost in a refactoring. Can look into bringing it back into the current code base. @mjpost, feel free to assign to me.

…

On Mon, Jun 8, 2020, 7:49 AM Martin Popel ***@***.***> wrote: Efficient implementation of bootstrap does not call sacrebleu 10000 times (doing the tokenization 10000 times), so a "wrapper command around sacrebleu" is not a proper solution. We could provide sacrebleu-bootstrap command, which would take multiple systems' translations on the input (and optionally their names as --system-names) and the number of resamples as a parameter, but I don't see any advantage over keeping this within sacrebleu. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#40 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACSH2DRRNSQTZPQVHSELADRVT26JANCNFSM4IHE5DYA> .

[martinpopel]

Hi @cfedermann, I linked your implementation (#11) above (so it is not completely lost), but it doesn't do paired bootstrap resampling, so I also linked a Python implementation of paired bootstrap (by a student of mine), which is actually built on top of sacrebleu.

[ozancaglayan]

Hi, I want to spend some time on sacrebleu during the following days and apparently there are enough materials around for integrating so I can try to look at this as well. There's also a recent PR about this, which you may want to review. An additional question is: I saw somewhere in a slidedeck that (paired) approximate randomization is a better method for MT than (paired) bootstrap resampling. What is your take on this?

[cfedermann]

I'd implement and offer both bootstrap and AR, that makes everybody happy...

…

On Mon, Jun 8, 2020, 12:56 PM Ozan Caglayan ***@***.***> wrote: Hi, I want to spend some time on sacrebleu during the following days and apparently there are enough materials around for integrating so I can try to look at this as well. There's also a recent PR about this, which you may want to review. An additional question is: I saw somewhere in a slidedeck that (paired) approximate randomization is a better method for MT than (paired) bootstrap resampling. What is your take on this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#40 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACSH2FYQQJVTPFNAWYP4D3RVU673ANCNFSM4IHE5DYA> .

[ozancaglayan]

The implementation of this feature requires the ability to provide N input files when running sacreBLEU. If we were to only use -i, this should have been easier but we also support reading from STDIN. I can't easily see a solution without breaking backward compatibility: we can read \t separated stream from STDIN and when --input - is given, but we can't use the same --input for nargs-style multiple arguments at the same time.

What are your thoughts on this? Maybe we can keep the API as the same but add --system-files and --system-names for significance test but I'm scared that this will pollute/confuse the codebase a lot.

[mjpost]

Why can't we use both at the same time? Couldn't we write an input wrapper that (a) receives args.input and returns a set of input streams? In the case that args.input == sys.stdin, it would split on tabs and return a stream.

We could also support this feature only with -i, if that were easier.

[ozancaglayan]

I think the main problem is related to argparse here, rather than STDIN-reading as I previously told.

--input was defined to be a str with a default of -. refs are defined to be a positional argument without a hyphen prefix and it expects to read 0 or more references i.e. nargs='*'. (I think this was mainly due to the fact that sacrebleu would always be providing references internally through --test-set.)

I don't think we can have two arguments with nargs='*'. This eliminates the easiest form of use i.e. --input rnn/hypotheses transformers/hypotheses smt/hypotheses. This leaves us with the only option of preparing everything as a tab separated file and giving it to --input, which is pretty cumbersome.

If I were to break compatibility, I would suggest to not have refs as positional arguments and instead use --refs with nargs='*' as well. (Although I need to check whether this runs without issue with argparse..)

sacrebleu -i rnn/hyps transformers/hyps smt/hyps -r refs
sacrebleu -i rnn/hyps transformers/hyps smt/hyps -t wmt16/dev -l en-de
sacrebleu -i rnn/hyps -r refs
sacrebleu -i - -r refs (with - being tsv file or single file)

Please correct me if I'm missing something.

EDIT:
Ok, converting --input to nargs='*' works only if refs is written before --input through the cmdline.

[martinpopel]

First, I think sacrebleu ref1 ref2 ref3 --input in1 in2 should work.
Second, paste in1 in2 | sacrebleu ref1 ref2 ref3 should not be cumbersome.
So I think we could continue supporting both --input and stdin, even with more input files.
Just in the case of tab-separated stdin, we perhaps should requite an option indicating how many input systems to expect, so that we can report errors with unexpected number of columns (similarly as we do with tab-separated references and -nr 2).