Skip to content

Conversation

sky-2002
Copy link
Contributor

This PR adds a fix for the issue mentioned in #1108

However I have a points to discuss @shahules786 :

  • I had added conciseness_score to penalize long summaries, but I also do not want to promote very very short and skimpy summaries, need to find a middle ground.
  • Is averaging a good way to combine QA_score and conciseness_score?
  • Ranking based metrics to measure quality of summarization (as mentioned by shahul in the above issue)

Given the conclusions we reach based on these discussion points, I will push more commits, let's keep this PR open till we resolve these points.

@dosubot dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Aug 15, 2024
@shahules786
Copy link
Member

shahules786 commented Aug 16, 2024

Hey @sky-2002 thanks for the PR, my friend. I have answered your query in the here
I think doing that will resolve the issue. Let me know your thoughts

When you make the change please make sure you modify and explain the same in docs too

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Aug 16, 2024
Copy link
Member

@shahules786 shahules786 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@shahules786
Copy link
Member

@sky-2002 Sorry for the late merge. There were few important changes I had to make on this, please take a look at it so you know.

@shahules786 shahules786 removed the request for review from jjmachan August 23, 2024 12:20
@shahules786
Copy link
Member

Hey @jjmachan Can you check what's with the lining errors on those files (not modified in this PR)

@jjmachan jjmachan linked an issue Aug 27, 2024 that may be closed by this pull request
@jjmachan jjmachan merged commit d58dc01 into explodinggradients:main Aug 27, 2024
@sky-2002 sky-2002 deleted the summarization/edge-case-fix branch November 28, 2024 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[R-279] [R-280] Summarization Score Formula is unreasonable

3 participants