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

FIx:Correction of the calculation of the MRR indicator in the RAG benchmark #1228

Merged
merged 1 commit into from
Apr 26, 2024

Conversation

YangQianli92
Copy link
Contributor

@YangQianli92 YangQianli92 commented Apr 26, 2024

User description

  1. When calculating the MRR metric, we only calculate the first correctly recalled countdown sort score and return that score

  2. It depends on the specifics of your document as to how to determine if the documents are being recalled correctly; here we use a direct determination of equality, but in fact this is not an optimal approach


Type

bug_fix


Description

  • Refactored the mean_reciprocal_rank function in base.py to improve the calculation logic:
    • Changed loop order to iterate nodes first, enhancing the logical flow.
    • Adjusted the condition to check for text match in documents.
    • Updated the return statement to immediately return upon match, simplifying the function.
    • Removed the averaging of mrr_sum which was previously conditional on reference_docs being non-empty.

Changes walkthrough

Relevant files
Bug fix
base.py
Refactor mean_reciprocal_rank Calculation Logic                   

metagpt/rag/benchmark/base.py

  • Updated the loop structure in mean_reciprocal_rank to iterate over
    nodes first and then reference documents.
  • Changed the condition to check if text is in doc.
  • Modified the return logic to immediately return mrr_sum upon finding a
    match.
  • Removed the conditional average calculation at the end of the
    function.
  • +6/-6     

    PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link

    PR Description updated to latest commit (a1e8018)

    Copy link

    PR Review

    ⏱️ Estimated effort to review [1-5]

    2, because the changes are localized to a single function with a clear intent, but require a careful review to ensure the logic is correctly implemented and no new bugs are introduced.

    🧪 Relevant tests

    No

    🔍 Possible issues

    Logic Error: The variable text in the condition if text in doc: is not defined in the provided context. It seems like it should be node.text.

    Early Return: The change to return mrr_sum immediately upon finding a match changes the behavior significantly. It now returns as soon as any match is found, which might not compute the intended mean reciprocal rank across all reference documents.

    🔒 Security concerns

    No


    ✨ Review tool usage guide:

    Overview:
    The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

    The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

    • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
    /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...
    
    [pr_reviewer]
    some_config1=...
    some_config2=...
    

    See the review usage page for a comprehensive guide on using this tool.

    Copy link

    PR Code Suggestions

    CategorySuggestions                                                                                                                                                       
    Bug
    Correct the variable reference to use the appropriate object attribute.

    Replace the variable text with node.text in the condition to correctly reference the text
    attribute of the node object.

    metagpt/rag/benchmark/base.py [119]

    -if text in doc:
    +if node.text in doc:
     
    Ensure the function completes all iterations before returning a result.

    Remove the return mrr_sum statement from within the loop to ensure the function calculates
    the MRR for all nodes and documents before returning the final sum.

    metagpt/rag/benchmark/base.py [121]

    -return mrr_sum
    +# This line should be removed.
     
    Prevent division by zero by checking if the reference document list is empty.

    Add a check to ensure that reference_docs is not empty before proceeding with the
    calculation to avoid division by zero.

    metagpt/rag/benchmark/base.py [117]

    +if not reference_docs:
    +    return 0.0
     for i, node in enumerate(nodes, start=1):
     
    Enhancement
    Normalize the MRR calculation to account for the number of documents.

    Normalize the MRR sum by dividing by the number of reference documents, if not empty, to
    maintain the original functionality of the method.

    metagpt/rag/benchmark/base.py [123]

    -return mrr_sum
    +return mrr_sum / len(reference_docs) if reference_docs else 0.0
     
    Performance
    Optimize the loop to reduce the number of iterations.

    Replace the nested loops with a more efficient approach to avoid unnecessary iterations
    and improve performance.

    metagpt/rag/benchmark/base.py [117-120]

     for i, node in enumerate(nodes, start=1):
    -    for doc in reference_docs:
    -        if node.text in doc:
    -            mrr_sum += 1.0 / i
    +    if any(node.text in doc for doc in reference_docs):
    +        mrr_sum += 1.0 / i
     

    ✨ Improve tool usage guide:

    Overview:
    The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

    • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
    /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...
    
    [pr_code_suggestions]
    some_config1=...
    some_config2=...
    

    See the improve usage page for a comprehensive guide on using this tool.

    @geekan geekan merged commit f201b2f into geekan:main Apr 26, 2024
    0 of 3 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants