[bugfix] Fix mbpp evaluater class missing logger #115
Conversation
Summary of ChangesHello @SJTUyh, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a bug in the MBPP evaluator classes by fixing two related initialization issues. It corrects a syntax error in the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly fixes a bug in MBPPEvaluator and MBPPPassKEvaluator where the parent class constructor was not being called, leading to issues like a missing logger. The syntax for the super().__init__() call is corrected, and a missing call is added. The changes are accurate and resolve the issue. I have one minor suggestion regarding documentation for the new parameter.
| """ | ||
|
|
||
| def __init__(self, k=(1, 10, 100)) -> None: | ||
| def __init__(self, k=(1, 10, 100), metric: str = 'MBPP') -> None: |
There was a problem hiding this comment.
With the addition of the metric parameter, the class docstring for MBPPPassKEvaluator is now out of sync. Please update the docstring to include documentation for the new metric parameter to improve code clarity and maintainability.
There was a problem hiding this comment.
Pull request overview
This PR fixes critical syntax errors in the MBPPPassKEvaluator class that prevented it from functioning properly. The main issue was a missing super().__init__() call that prevented the evaluator from properly inheriting logger functionality from the parent MBPPEvaluator class.
Changes:
- Fixed incorrect
super.__init__()syntax to propersuper().__init__()call - Corrected debug log messages to use the correct
task_idvariable instead of undefinedindex - Added
metricparameter toMBPPPassKEvaluatorconstructor to allow passing metric type to parent class
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| ais_bench/benchmark/datasets/mbpp.py | Fixed syntax errors in MBPPEvaluator and MBPPPassKEvaluator classes including super().init() call, variable references in logger statements, and added metric parameter support |
| .github/workflows/run-ut-on-pr-py.yml | Added 'run' label to CI runner configuration (unrelated to the bugfix) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """Better use for pass k evaluation. | ||
| Args: | ||
| k(Tuple[int]): Choices of Pass@k. Defaults to (1, 10, 100) |
There was a problem hiding this comment.
The docstring should be updated to document the new 'metric' parameter that was added to the constructor. Include the parameter type, description, and default value to maintain consistency with documentation standards.
| k(Tuple[int]): Choices of Pass@k. Defaults to (1, 10, 100) | |
| k(Tuple[int]): Choices of Pass@k. Defaults to (1, 10, 100). | |
| metric (str): Name of the evaluation metric. Defaults to 'MBPP'. |
| def __init__(self, k=(1, 10, 100), metric: str = 'MBPP') -> None: | ||
| if not isinstance(k, Sequence): | ||
| k = (k, ) | ||
| self.k = k | ||
| super().__init__(metric=metric) |
There was a problem hiding this comment.
The new metric parameter lacks test coverage. Consider adding test cases to verify that the metric parameter is correctly passed to the parent MBPPEvaluator class and that both 'MBPP' and 'MBPPPlus' values work correctly with MBPPPassKEvaluator.
| jobs: | ||
| pr_run_test: | ||
| runs-on: [self-hosted, Linux] | ||
| runs-on: [self-hosted, Linux, run] |
There was a problem hiding this comment.
This workflow configuration change appears unrelated to the bugfix described in the PR title and description. The PR is specifically about fixing syntax errors in the MBPPPass@kEvaluator class, but this change modifies CI/CD runner configuration. Consider removing this change or creating a separate PR for infrastructure updates.
| runs-on: [self-hosted, Linux, run] | |
| runs-on: ubuntu-latest |
2 participants
Thanks for your contribution; we appreciate it a lot. The following instructions will make your pull request healthier and help you get feedback more easily. If you do not understand some items, don't worry, just make the pull request and seek help from maintainers.
感谢您的贡献，我们非常重视。以下说明将使您的拉取请求更健康，更易于获得反馈。如果您不理解某些项目，请不要担心，只需提交拉取请求并从维护人员那里寻求帮助即可。
PR Type / PR类型
Related Issue | 关联 Issue
Fixes #(issue ID / issue 编号) / Relates to #(issue ID / issue 编号)
#114
🔍 Motivation / 变更动机
Please describe the motivation of this PR and the goal you want to achieve through this PR.
请描述您的拉取请求的动机和您希望通过此拉取请求实现的目标。
There some basic syntax errors in MBPPPass@kEvaluator class.
📝 Modification / 修改内容
Please briefly describe what modification is made in this PR.
请简要描述此拉取请求中进行的修改。
Fix some basic syntax errors in MBPPPass@kEvaluator class
📐 Associated Test Results / 关联测试结果
ais_bench --models vllm_api_stream_chat --datasets mbpp_passk_gen_3_shot_chat_prompt --mode eval --debug --reuse 20260119_113305Does the modification introduce changes that break the backward compatibility of the downstream repositories? If so, please describe how it breaks the compatibility and how the downstream projects should modify their code to keep compatibility with this PR.
是否引入了会破坏下游存储库向后兼容性的更改？如果是，请描述它如何破坏兼容性，以及下游项目应该如何修改其代码以保持与此 PR 的兼容性。
If the modification introduces performance degradation, please describe the impact of the performance degradation and the expected performance improvement.
如果引入了性能下降，请描述性能下降的影响和预期的性能改进。
🌟 Use cases (Optional) / 使用案例（可选）
If this PR introduces a new feature, it is better to list some use cases here and update the documentation.
如果此拉取请求引入了新功能，最好在此处列出一些用例并更新文档。
✅ Checklist / 检查列表
Before PR:
After PR:
👥 Collaboration Info / 协作信息
🌟 Useful CI Command / 实用的CI命令
/gemini review/gemini summary/gemini help/readthedocs build