IWSLT'14 Results using ESPnet2-MT

[pyf98]

Hi, I tried some configs for IWSLT'14 De-En using ESPnet2-MT. I followed the standard Transformer config in fairseq, but the batchifying method is different. I think my effective batch size is a few times larger than fairseq, and my learning rate and warmup steps are also larger. The current script for BLEU calculation is different from the standard script for this dataset.

The best result is reported in README.md. Besides, I added two options in mt/espnet_model.py to allow weight tying of the input embedding and the output linear layer in encoder and decoder.

We probably need to test the implementation further on other datasets.

[pyf98]

Hi @siddalmia @ftshijt @brianyan918 Could you check this PR? If the changes are reasonable, we can continue the experiments.

[brianyan918]

Looks good! I believe your initial result on this dataset is reasonable as well.

[codecov]

Codecov Report

Merging #4132 (11e3e7c) into master (a04a98c) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #4132   +/-   ##
=======================================
  Coverage   80.43%   80.43%           
=======================================
  Files         442      442           
  Lines       38557    38557           
=======================================
  Hits        31015    31015           
  Misses       7542     7542           

Flag	Coverage Δ
test_integration_espnet1	67.13% <ø> (ø)
test_integration_espnet2	51.14% <ø> (ø)
test_python	66.51% <ø> (ø)
test_utils	24.45% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a04a98c...11e3e7c. Read the comment docs.

[siddalmia]

Perfect thank you @pyf98 ! Some minor comments -

1. Readme - Can you mention Blue-4 and also mention if the scoring was done on detokenized cased or uncased or tokenized outputs
2. The fix that you made in line 515 of mt.sh also exists in st.sh. Can you also fix it there as well?

[sw005320]

I leave detailed reviews by @siddalmia @ftshijt @brianyan918.
My comment is that we should add this result to https://github.com/espnet/espnet/blob/master/README.md#mt-results

[pyf98]

Thanks for your comments. I'm fixing them now.

BTW, I have a question about the generated hypotheses. In some examples (131 out of 6750 in the test set), there is a trailing <sos/eos> as shown below:

They &apos;re even hurting .<sos/eos>	(They-utt000382)
I &apos;m a writer .<sos/eos>	(I-utt000553)
Thank you .<sos/eos>	(Thank-utt000722)

If we directly use this result to calculate BLEU, will it adversely affect the performance?

[brianyan918]

I think it would. Not sure that sacrebleu knows to ignore that.

…

On Fri, Mar 4, 2022 at 3:36 PM Yifan Peng ***@***.***> wrote: Thanks for your comments. I'm fixing them now. BTW, I have a question about the generated hypotheses. In some examples, there is a trailing <sos/eos> as shown below: They &apos;re even hurting .<sos/eos> (They-utt000382) I &apos;m a writer .<sos/eos> (I-utt000553) Thank you .<sos/eos> (Thank-utt000722) If we directly use this result to calculate BLEU, will it adversely affect the performance? — Reply to this email directly, view it on GitHub <#4132 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACQBBOOJHAO7Q3IPYF5DCZTU6JX3ZANCNFSM5P6JNJOA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[mergify]

This pull request is now in conflict :(

[pyf98]

Hi @siddalmia, about the second point, I checked the st.sh for the token_joint part (https://github.com/espnet/espnet/blob/master/egs2/TEMPLATE/st1/st.sh#L745).

It seems that the source text is not combined with the target text if token_joint is true, so the same problem doesn't exist. But I'm not sure if this is the expected behavior for ST or this is actually a bug. Maybe @ftshijt could compare mt.sh and st.sh for the generation of token_list.

[pyf98]

I have another question about the encoder input embedding layer. Where should we put the embedding layer, which is a torch.nn.Embedding instance followed by pos_enc_class? According to the current implementation, there seem to be two options:

1. Put it in the frontend (Sid added a new type of frontend as embedding, and also modified transformer_encoder). But currently it only supports transformer encoder and absolute positional encoding.
2. Put it in the embed of encoder. This requires some modifications on mt/espnet_model.py.

Current results and the weight tying implementation are based on the first option.

[ftshijt]

Hi @siddalmia, about the second point, I checked the st.sh for the token_joint part (https://github.com/espnet/espnet/blob/master/egs2/TEMPLATE/st1/st.sh#L745).

It seems that the source text is not combined with the target text if token_joint is true, so the same problem doesn't exist. But I'm not sure if this is the expected behavior for ST or this is actually a bug. Maybe @ftshijt could compare mt.sh and st.sh for the generation of token_list.

I think it is actually a bug to be fixed, could you help me to combine the tgt_text ahead of time?

[ftshijt]

Since the PR already looks great (and also could be beneficial to upcoming deadlines), I will merge the PR now. @pyf98 could you also reflect the bug in ST implementation in another PR?

[pyf98]

Cool! Thanks @ftshijt

[pyf98]

Hi @ftshijt I made a new PR for the ST issue: #4143