[feature] [sub feature 3] Support qwen Image edit infer with gedit dataset #150
[feature] [sub feature 3] Support qwen Image edit infer with gedit dataset #150SJTUyh wants to merge 5 commits intoAISBench:masterfrom
Conversation
SJTUyh
changed the title
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 significantly expands the benchmarking capabilities by introducing full support for the Qwen Image Edit model, particularly for image editing tasks on 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. 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
This pull request introduces significant new functionality for image editing with the Qwen model and a judging mechanism for evaluating model outputs. However, a critical insecure deserialization vulnerability using pickle.loads in the distributed coordinator could lead to remote code execution. Additionally, several instances of path traversal were found in the worker post-processing logic, which could allow for arbitrary file deletion or overwrite of .jsonl files. Beyond these security concerns, I've identified areas for improvement related to robustness, code clarity, and best practices, including making task merging more reliable, ensuring file operations are atomic, and improving code consistency by removing mixed-language comments. These issues should be addressed before merging.
I am having trouble creating individual review comments. Click here to see my feedback.
ais_bench/benchmark/models/local_models/qwenimage_edit/distributed/group_coordinator.py (411)
The recv_object method uses pickle.loads to deserialize data received from other ranks in a distributed environment. pickle is inherently insecure and can be exploited to execute arbitrary code if an attacker can control the data sent over the distributed communication channel. While this occurs in a distributed environment, using pickle for network communication is a significant security risk, especially if any node in the cluster is compromised, as it allows for lateral movement and remote code execution (RCE) across the cluster.
Recommendation: Replace pickle with a secure serialization format such as json or orjson. If complex Python objects must be transferred, consider using a safer alternative or implementing strict validation of the deserialized data.
ais_bench/benchmark/models/local_models/base.py (60)
The method signature for generate has been changed from _generate(self, input, ...) to generate(self, inputs, ...). However, the base class BaseModel still has an abstract method _generate. This should be changed to generate to match the new signature in BaseLMModel and avoid potential NotImplementedError issues in subclasses.
ais_bench/benchmark/models/local_models/qwen_image_edit_mindie_sd.py (77)
The DEFAULT_NUM_INFERENCE_STEPS is set to 1, with the original value of 40 commented out. A single inference step is unusually low for a diffusion model and will likely produce very low-quality images. This might be for quick testing, but it's a risky default. If this is for debugging, consider making it more explicit or using a higher default value.
ais_bench/benchmark/cli/workers.py (218-223)
This section is vulnerable to path traversal, as file paths are constructed using unsanitized model and dataset abbreviations (abbr). An attacker could use path traversal sequences (e.g., ../) to delete arbitrary .jsonl files outside the intended directory. Additionally, removing the original prediction file before writing the new one is risky; if the dump_jsonl operation fails, the original data will be lost. Consider sanitizing abbr values and using atomic file operations (write to a temporary file then move) to prevent both path traversal and data loss.
tmp_judge_org_prediction_path = judge_org_prediction_path + '.tmp'
dump_jsonl(judge_preds, tmp_judge_org_prediction_path)
shutil.move(tmp_judge_org_prediction_path, judge_org_prediction_path)ais_bench/benchmark/cli/workers.py (286-293)
The Eval worker constructs file paths using unsanitized model and dataset abbreviations, leading to a path traversal vulnerability. This could allow overwriting arbitrary .jsonl files via shutil.copy. It's crucial to sanitize all abbr values and restrict paths to the intended output directory. Additionally, please translate the Chinese comment on line 292 to English for consistency and readability.
# Copy the file from cur_results_path to final_org_results_pathais_bench/benchmark/cli/workers.py (176-182)
Using str() on a dictionary to generate a key for grouping is not robust. The string representation of a dictionary is not guaranteed to be consistent if the key order changes. A more reliable method is to use json.dumps with sort_keys=True to create a canonical string representation of the dictionary.
key = (
task["models"][0]["abbr"] # same model
+ "_"
+ str(task['datasets'][0][0]['type']) # same dataset type
+ "_"
+ json.dumps(task["datasets"][0][0]["infer_cfg"]["inferencer"], sort_keys=True) # same inferencer with the same args
)ais_bench/benchmark/models/local_models/base.py (136-146)
The generate_from_template method has been removed. While it might not be used currently, this could be a breaking change for other parts of the codebase that might rely on it. Please ensure this removal is intentional and all call sites have been updated. If it's no longer needed, this is fine, but it's a significant removal worth double-checking.
ais_bench/benchmark/models/local_models/qwen_image_edit_mindie_sd.py (223-224)
There are print statements here and in other places in this file (e.g., line 250). These should be replaced with proper logging using self.logger for better log management, especially to control log levels and output destinations in production environments. Print statements can clutter the output and are hard to disable.
self.logger.debug(f"in _generate")
self.logger.debug(f"输入: {input}")
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 / 变更动机
Please describe the motivation of this PR and the goal you want to achieve through this PR.
请描述您的拉取请求的动机和您希望通过此拉取请求实现的目标。
📝 Modification / 修改内容
Please briefly describe what modification is made in this PR.
请简要描述此拉取请求中进行的修改。
📐 Associated Test Results / 关联测试结果
Please provide links to the related test results, such as CI pipelines, test reports, etc.
请提供相关测试结果的链接，例如 CI 管道、测试报告等。
Does 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