[template] remove compat and update docs#7192
Conversation
Summary of ChangesHello @Jintao-Huang, 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 focuses on enhancing the flexibility and clarity of how rejected responses are handled within 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. 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
|
There was a problem hiding this comment.
Code Review
This pull request removes some compatibility code related to rejected_response and updates the documentation to reflect new data formats. The changes in the Python code are a good refactoring, but I found a small bug in an assertion. The documentation updates are helpful, but there are some inconsistencies and misleading comments that I've pointed out. Overall, good changes that improve code clarity and document new features.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request focuses on removing compatibility code related to rejected_response and updating the documentation to reflect new, more flexible data formats. The changes in swift/llm/template/template_inputs.py effectively refactor the handling of rejected_response to support both string and list-of-message formats, which is a good improvement. The documentation updates in both Chinese and English are clear and provide helpful examples for the new features. I've identified a minor typo in the Chinese documentation and suggest an improvement for input validation in the Python code by using ValueError instead of assert for better robustness. Overall, this is a solid pull request that enhances code clarity and documentation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request effectively removes legacy compatibility code for rejected_response and updates the documentation to reflect new data format capabilities, particularly for DPO/RLHF scenarios. The code changes in swift/llm/template/template_inputs.py are well-executed, introducing clearer logic for handling rejected_response as either a string or a list of messages, which aligns with the documentation. The documentation updates in both Chinese and English are clear and provide valuable examples. I've made two minor suggestions to improve formatting consistency in the markdown files.
| > 注: RM 额外支持 margin 列,参考[RM文档](../Instruction/RLHF.md#rm) | ||
|
|
||
| 当然,你也可以直接使用`rejected_messages`,而不是只提供`rejected_response`/`rejected_images`(需ms-swift>=3.8),这将提供更大的灵活度(例如多模态/agent场景)。若使用rejected_messages,在多模态场景下,你需要额外传入"rejected_images","rejected_audios","rejected_videos"等内容;在Agent场景下,你需要额外传入"rejected_tools"等内容。多模态数据格式例子如下: | ||
| - 若使用`rejected_response`,'rejected_images/rejected_audios/rejected_videos/rejected_tools'的默认值为'images/audios/videos/tools';若使用`rejected_messages`,则需要额外传入。 |
There was a problem hiding this comment.
为了保持文档格式的一致性,建议将 'rejected_images/audios/videos/tools' 和 'images/audios/videos/tools' 也使用反引号()包裹,就像文档中其他字段名(如 rejected_response`)一样。
| - 若使用`rejected_response`,'rejected_images/rejected_audios/rejected_videos/rejected_tools'的默认值为'images/audios/videos/tools';若使用`rejected_messages`,则需要额外传入。 | |
| - 若使用`rejected_response`,`rejected_images/audios/videos/tools`的默认值为`images/audios/videos/tools`;若使用`rejected_messages`,则需要额外传入。 |
| > Note: RM additionally supports the margin column. For details, refer to the [RM documentation](../Instruction/RLHF.md#rm). | ||
|
|
||
| Sure, you can also directly use `rejected_messages` instead of only providing `rejected_response` / `rejected_images` (requires ms-swift>=3.8), which offers greater flexibility (e.g., for multimodal or agent scenarios). If you use "rejected_messages", then in multimodal scenarios you must also provide "rejected_images", "rejected_audios", "rejected_videos", etc.; in Agent scenarios you must also provide "rejected_tools", etc. An example of the multimodal data format is as follows: | ||
| - If using `rejected_response`, the default values for 'rejected_images/rejected_audios/rejected_videos/rejected_tools' are 'images/audios/videos/tools'; if using `rejected_messages`, they need to be passed in additionally. |
There was a problem hiding this comment.
For consistency with how other field names like rejected_response are formatted in the document, it's recommended to also wrap 'rejected_images/audios/videos/tools' and 'images/audios/videos/tools' in backticks (`).
| - If using `rejected_response`, the default values for 'rejected_images/rejected_audios/rejected_videos/rejected_tools' are 'images/audios/videos/tools'; if using `rejected_messages`, they need to be passed in additionally. | |
| - If using `rejected_response`, the default values for `rejected_images/audios/videos/tools` are `images/audios/videos/tools`; if using `rejected_messages`, they need to be passed in additionally. |
No description provided.