Conversation
Summary of ChangesHello @GaoHuaZhang, 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! 此拉取请求旨在通过引入针对 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. Changelog
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
这个PR为math和agieval数据集添加了冒烟测试用例，这对于保证代码质量非常有帮助。在审查代码时，我注意到在accuracy_agieval和accuracy_math两个测试用例中，run.sh和clean.sh脚本存在大量的代码重复。为了提高代码的可维护性，强烈建议将这些脚本中的通用逻辑提取到共享脚本中。例如，可以创建一个通用的run.sh，由各个测试用例的run.sh来设置特定变量并调用它。这可以减少冗余代码，并使未来的修改更加容易。除此之外，我还针对run.sh中的重复代码块和clean.sh的简化提出了一些具体的建议。
| @@ -0,0 +1,4 @@ | |||
| #!/bin/bash | |||
| CUR_DIR=$(dirname $(readlink -f $0)) | |||
| [ -f "${CUR_DIR}/tmplog.txt" ] && rm -f "${CUR_DIR}/tmplog.txt" | |||
There was a problem hiding this comment.
此行命令可以简化。rm -f 命令在文件不存在时不会报错，因此前面的 [ -f ... ] 检查是多余的。
另外，这个 clean.sh 脚本与 accuracy_math 测试用例中的脚本内容完全相同。为了避免代码重复，建议将此脚本统一管理，例如创建一个公共的清理脚本供所有测试用例使用。
| [ -f "${CUR_DIR}/tmplog.txt" ] && rm -f "${CUR_DIR}/tmplog.txt" | |
| rm -f "${CUR_DIR}/tmplog.txt" |
| if [ ! -f "$LOG_EVAL_OUTPUT_PATH" ];then | ||
| echo "Can't find $LOG_EVAL_OUTPUT_PATH" | ||
| exit $ret_failed | ||
| fi | ||
| if [ ! -f "$LOG_INFER_OUTPUT_PATH" ];then | ||
| echo "Can't find $LOG_INFER_OUTPUT_PATH" | ||
| exit $ret_failed | ||
| fi | ||
| if [ ! -f "$PREDICTIONS_OUTPUT_PATH" ];then | ||
| echo "Can't find $PREDICTIONS_OUTPUT_PATH" | ||
| exit $ret_failed | ||
| fi | ||
| if [ ! -f "$RESULTS_OUTPUT_PATH" ];then | ||
| echo "Can't find $RESULTS_OUTPUT_PATH" | ||
| exit $ret_failed | ||
| fi | ||
| if [ ! -f "$SUMMARY_OUTPUT_PATH" ];then | ||
| echo "Can't find $SUMMARY_OUTPUT_PATH" | ||
| exit $ret_failed | ||
| fi |
There was a problem hiding this comment.
这部分代码中存在大量重复的 if 语句来检查文件是否存在。为了提高代码的可读性和可维护性，建议使用一个循环来代替这些重复的检查。
| if [ ! -f "$LOG_EVAL_OUTPUT_PATH" ];then | |
| echo "Can't find $LOG_EVAL_OUTPUT_PATH" | |
| exit $ret_failed | |
| fi | |
| if [ ! -f "$LOG_INFER_OUTPUT_PATH" ];then | |
| echo "Can't find $LOG_INFER_OUTPUT_PATH" | |
| exit $ret_failed | |
| fi | |
| if [ ! -f "$PREDICTIONS_OUTPUT_PATH" ];then | |
| echo "Can't find $PREDICTIONS_OUTPUT_PATH" | |
| exit $ret_failed | |
| fi | |
| if [ ! -f "$RESULTS_OUTPUT_PATH" ];then | |
| echo "Can't find $RESULTS_OUTPUT_PATH" | |
| exit $ret_failed | |
| fi | |
| if [ ! -f "$SUMMARY_OUTPUT_PATH" ];then | |
| echo "Can't find $SUMMARY_OUTPUT_PATH" | |
| exit $ret_failed | |
| fi | |
| for output_file in \ | |
| "$LOG_EVAL_OUTPUT_PATH" \ | |
| "$LOG_INFER_OUTPUT_PATH" \ | |
| "$PREDICTIONS_OUTPUT_PATH" \ | |
| "$RESULTS_OUTPUT_PATH" \ | |
| "$SUMMARY_OUTPUT_PATH" | |
| do | |
| if [ ! -f "$output_file" ]; then | |
| echo "Can't find $output_file" | |
| exit $ret_failed | |
| fi | |
| done |
| @@ -0,0 +1,4 @@ | |||
| #!/bin/bash | |||
| CUR_DIR=$(dirname $(readlink -f $0)) | |||
| [ -f "${CUR_DIR}/tmplog.txt" ] && rm -f "${CUR_DIR}/tmplog.txt" | |||
There was a problem hiding this comment.
此行命令可以简化。rm -f 命令在文件不存在时不会报错，因此前面的 [ -f ... ] 检查是多余的。
另外，这个 clean.sh 脚本与 accuracy_agieval 测试用例中的脚本内容完全相同。为了避免代码重复，建议将此脚本统一管理，例如创建一个公共的清理脚本供所有测试用例使用。
| [ -f "${CUR_DIR}/tmplog.txt" ] && rm -f "${CUR_DIR}/tmplog.txt" | |
| rm -f "${CUR_DIR}/tmplog.txt" |
| if [ ! -f "$LOG_EVAL_OUTPUT_PATH" ];then | ||
| echo "Can't find $LOG_EVAL_OUTPUT_PATH" | ||
| exit $ret_failed | ||
| fi | ||
| if [ ! -f "$LOG_INFER_OUTPUT_PATH" ];then | ||
| echo "Can't find $LOG_INFER_OUTPUT_PATH" | ||
| exit $ret_failed | ||
| fi | ||
| if [ ! -f "$PREDICTIONS_OUTPUT_PATH" ];then | ||
| echo "Can't find $PREDICTIONS_OUTPUT_PATH" | ||
| exit $ret_failed | ||
| fi | ||
| if [ ! -f "$RESULTS_OUTPUT_PATH" ];then | ||
| echo "Can't find $RESULTS_OUTPUT_PATH" | ||
| exit $ret_failed | ||
| fi | ||
| if [ ! -f "$SUMMARY_OUTPUT_PATH" ];then | ||
| echo "Can't find $SUMMARY_OUTPUT_PATH" | ||
| exit $ret_failed | ||
| fi |
There was a problem hiding this comment.
这部分代码中存在大量重复的 if 语句来检查文件是否存在。为了提高代码的可读性和可维护性，建议使用一个循环来代替这些重复的检查。这个问题在 accuracy_agieval/run.sh 中也同样存在。
| if [ ! -f "$LOG_EVAL_OUTPUT_PATH" ];then | |
| echo "Can't find $LOG_EVAL_OUTPUT_PATH" | |
| exit $ret_failed | |
| fi | |
| if [ ! -f "$LOG_INFER_OUTPUT_PATH" ];then | |
| echo "Can't find $LOG_INFER_OUTPUT_PATH" | |
| exit $ret_failed | |
| fi | |
| if [ ! -f "$PREDICTIONS_OUTPUT_PATH" ];then | |
| echo "Can't find $PREDICTIONS_OUTPUT_PATH" | |
| exit $ret_failed | |
| fi | |
| if [ ! -f "$RESULTS_OUTPUT_PATH" ];then | |
| echo "Can't find $RESULTS_OUTPUT_PATH" | |
| exit $ret_failed | |
| fi | |
| if [ ! -f "$SUMMARY_OUTPUT_PATH" ];then | |
| echo "Can't find $SUMMARY_OUTPUT_PATH" | |
| exit $ret_failed | |
| fi | |
| for output_file in \ | |
| "$LOG_EVAL_OUTPUT_PATH" \ | |
| "$LOG_INFER_OUTPUT_PATH" \ | |
| "$PREDICTIONS_OUTPUT_PATH" \ | |
| "$RESULTS_OUTPUT_PATH" \ | |
| "$SUMMARY_OUTPUT_PATH" | |
| do | |
| if [ ! -f "$output_file" ]; then | |
| echo "Can't find $output_file" | |
| exit $ret_failed | |
| fi | |
| done |
1 participant
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 编号)
🔍 Motivation / 变更动机
为 agieval、math 两个精度评测场景补充 smoke test，便于在 CI 中快速回归 run_server_accuracy / llm_datasets_main 流程，验证配置与环境正确性。
Add smoke tests for agieval and math accuracy evaluation scenarios to enable quick regression of run_server_accuracy / llm_datasets_main in CI and verify config and environment.
📝 Modification / 修改内容
accuracy_agieval：agieval 数据集（冒烟仅跑子集 agieval-gaokao-chinese），vllm-api-general-chat。accuracy_math：math 数据集（子集 math_prm800k_500），vllm-api-general-chat。case.yml、run.sh、clean.sh，以及ais_bench_configs/下 datasets、models（vllm_api）配置；目录分别为smoke_tests/test-case/run_server_accuracy/llm_datasets_main/accuracy_agieval/与accuracy_math/。ais_bench/benchmark/configs/datasets/与ais_bench/benchmark/configs/models/的现有配置，本次为拷贝到 smoke 用例目录并接好 run/clean，无修改既有业务逻辑。📐 Associated Test Results / 关联测试结果
待 CI 运行后补充。 / To be added after CI run.
无。仅新增 smoke test 用例，不涉及下游兼容性。 / None. New smoke test cases only; no downstream compatibility impact.
无。 / None.
🌟 Use cases (Optional) / 使用案例（可选）
✅ Checklist / 检查列表
Before PR:
After PR:
👥 Collaboration Info / 协作信息
🌟 Useful CI Command / 实用的CI命令
/gemini review/gemini summary/gemini help/readthedocs build