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

No difference between RougeL and RougeLsum #904

Closed
FeiWang96 opened this issue Mar 23, 2022 · 2 comments · Fixed by #944
Closed

No difference between RougeL and RougeLsum #904

FeiWang96 opened this issue Mar 23, 2022 · 2 comments · Fixed by #944
Assignees
Labels
bug / fix Something isn't working help wanted Extra attention is needed topic: Text
Milestone

Comments

@FeiWang96
Copy link

🐛 Bug

To Reproduce

I finetune BART on CNN/DailyMail dataset using lightning transformers.

Code sample

https://github.com/PyTorchLightning/lightning-transformers/blob/master/lightning_transformers/task/nlp/summarization/model.py#L43

Expected behavior

Each document and summary in CNN/DailyMail have multiple sentences, so RougeL and RougeLsum should be different.

Environment

  • OS (e.g., Linux): Linux
  • Python & PyTorch Version (e.g., 1.0): Python 3.9, Pytorch 1.10
  • How you installed PyTorch (conda, pip, build command if you used source): pip
  • Any other relevant information:

Additional context

I've checked the code for computing RougeL and RougeLsum.
It seems that the only difference is that a separator '\n' is inserted after each sentence when calculating RougeLsum.
However, the '\n' is not treated as a special token when calculating lcs and the final score.

@FeiWang96 FeiWang96 added bug / fix Something isn't working help wanted Extra attention is needed labels Mar 23, 2022
@github-actions
Copy link

Hi! thanks for your contribution!, great first issue!

@SkafteNicki
Copy link
Member

cc: @stancld

@SkafteNicki SkafteNicki added this to the v0.8 milestone Mar 23, 2022
@Borda Borda added this to the v0.8 milestone May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug / fix Something isn't working help wanted Extra attention is needed topic: Text
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants