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

[Tokenizer]Add Chat template #8226

Merged
merged 21 commits into from
Apr 28, 2024
Merged

Conversation

Southpika
Copy link
Contributor

PR types

Function optimization

PR changes

APIs

Description

Add chat_template in config file to load template

Copy link

paddle-bot bot commented Apr 3, 2024

Thanks for your contribution!

@Southpika
Copy link
Contributor Author

移除encode_chat_inputs中对于system的单独处理,并入第一轮对话中合并(作为不可学习的内容)

Copy link

codecov bot commented Apr 7, 2024

Codecov Report

Attention: Patch coverage is 87.64045% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 55.36%. Comparing base (cb9ba2d) to head (2124df7).
Report is 2 commits behind head on develop.

❗ Current head 2124df7 differs from pull request most recent head 7d60cff. Consider uploading reports for the commit 7d60cff to get more accurate results

Files Patch % Lines
paddlenlp/transformers/tokenizer_utils.py 87.20% 11 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #8226      +/-   ##
===========================================
- Coverage    55.37%   55.36%   -0.01%     
===========================================
  Files          613      614       +1     
  Lines        95870    95412     -458     
===========================================
- Hits         53084    52824     -260     
+ Misses       42786    42588     -198     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@PaddlePaddle PaddlePaddle locked and limited conversation to collaborators Apr 8, 2024
@PaddlePaddle PaddlePaddle unlocked this conversation Apr 8, 2024
@zjjlivein zjjlivein closed this Apr 8, 2024
@zjjlivein zjjlivein reopened this Apr 8, 2024
Copy link
Contributor

@wj-Mcat wj-Mcat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

你可能还需要做以下几个事情:

  1. 更新所有支持 chat-template 的 tokenizer_config.json
  2. 测试一下先有的多轮对话训练、推理等流程,保证全流程正确。

paddlenlp/transformers/tokenizer_utils.py Show resolved Hide resolved
paddlenlp/transformers/tokenizer_utils.py Show resolved Hide resolved
paddlenlp/transformers/tokenizer_utils.py Outdated Show resolved Hide resolved
) -> str | dict[str, numpy.ndarray | paddle.Tensor]:
if isinstance(conversation, str):
conversations = [{"role": "user", "content": conversation}]
elif isinstance(conversation, list):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

此外,也测试过新旧 chat_template 在 Predictor 中的使用是否符合预期,同时还要测试一下 gradio_ui 能够使用新旧 chat_template。

paddlenlp/transformers/tokenizer_utils.py Outdated Show resolved Hide resolved
@@ -692,6 +753,70 @@ def encode_chat_inputs(self, conversations: List[List[str, str]], context_data:
result["conversations"] = conversation_ids
return result

def _encode_chat_inputs(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果脱离了之前设计的训推一体的 ChatTemplate,这个函数的适用性应该还挺低的,根本用不了。

所以,不太建议将 encode_chat_inputs 这块逻辑写到 tokenizer 里面去,尽量写到前处理里面去。

所以,这块的调整可能 就比较大了。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

考虑到目前encode_chat_inputs函数使用较广,移除造成影响范围可能较大。是否可以考虑以下策略:
默认tgt src切分方式为 src中不含有bot start token:即tgt中含有完整的user轮+bot start token
如果需要重写,则在tokenizer类中单独定义:如qwen

paddlenlp/transformers/tokenizer_utils.py Outdated Show resolved Hide resolved
paddlenlp/transformers/tokenizer_utils.py Show resolved Hide resolved
paddlenlp/transformers/tokenizer_utils.py Outdated Show resolved Hide resolved
paddlenlp/transformers/tokenizer_utils.py Outdated Show resolved Hide resolved
paddlenlp/transformers/tokenizer_utils.py Outdated Show resolved Hide resolved
paddlenlp/transformers/tokenizer_utils.py Outdated Show resolved Hide resolved
paddlenlp/transformers/tokenizer_utils.py Outdated Show resolved Hide resolved
paddlenlp/transformers/utils.py Outdated Show resolved Hide resolved
@wawltor wawltor merged commit 71cc404 into PaddlePaddle:develop Apr 28, 2024
6 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants