Bugfix Summarization Score #1164
Conversation
dosubot
bot
added
the
size:S
There was a problem hiding this comment.
@emreds thanks a lot for pushing this PR and helping us Improve Ragas. I was however not sure if an assertion here is the best flow.
it will break the user code right if the assertions fail? Ideally the behaviour we want is to use logger.error() to print out an explainable error and then return a Nan value (or throw and exception since executor should catch it).
what do you think?
|
@jjmachan sure, makes more sense. I will update it. |
dosubot
bot
added
size:M
dosubot
bot
added
size:S
dosubot
bot
added
size:L
|
@emreds I made some small changes for fixing the type error but now it still returns an empty list. This is so that the types make sense. but I'm not sure if it logically correct |
|
I think it looks good, as you have mentioned it doesn't break the code but throws the error to understand the context. |
Hi! I was experimenting with Summarization Score and found an issue that when there are no questions generated by the LLM, it throws division by zero error due to 0 generated answers. ## Notable Changes - Added an assertion to keyphrases, questions and answers generation functions. - Changed the llm response variable name `answer` to `response` to prevent possible confusion with score related `answer`. Please let me know if there is something I can add. Cheers! --------- Co-authored-by: jjmachan <jamesjithin97@gmail.com>
Hi!
I was experimenting with Summarization Score and found an issue that when there are no questions generated by the LLM, it throws division by zero error due to 0 generated answers.
Notable Changes
answertoresponseto prevent possible confusion with score relatedanswer.Please let me know if there is something I can add.
Cheers!