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

集成Yi模型 #343

Merged
merged 10 commits into from
Aug 28, 2024
Merged

集成Yi模型 #343

merged 10 commits into from
Aug 28, 2024

Conversation

Haijian06
Copy link
Contributor


name: 集成Yi模型
about: Create a pull request

Description

你好👋,我在你们的model里面集成了Yi模型,修改了init文件

Checklist

Please check the following items before code is ready to be reviewed.

  • Code has passed all tests
  • Code is ready for review

@DavdGao
Copy link
Collaborator

DavdGao commented Jul 17, 2024

Please execute pre-commit run --all-files to reformat the code

@Haijian06
Copy link
Contributor Author

谢谢我已经根据要求修改了相关格式和代码,可以继续接下的工作了

@Haijian06
Copy link
Contributor Author

Hi @DavdGao, could you please help me review this?

@DavdGao
Copy link
Collaborator

DavdGao commented Jul 30, 2024

Hi @DavdGao, could you please help me review this?

Thanks for your contribution. We will give feedback as soon as possible.

Copy link
Collaborator

@DavdGao DavdGao left a comment

Choose a reason for hiding this comment

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

Please see inline comments, and does the yi API support streaming mode? Maybe we can add streaming mode in this PR as well.

src/agentscope/models/yi_model.py Outdated Show resolved Hide resolved
@zhijianma
Copy link
Collaborator

zhijianma commented Jul 30, 2024

Please complete the following section:

  • Add model_config_template to show how to use Yi Model.
  • Add UnitTest

@Haijian06
Copy link
Contributor Author

I've updated the code according to the rules, thank you @DavdGao @zhijianma. I'm unsure about the best place to put the demo and UnitTest. Would you mind advising where they should go?

@garyzhang99
Copy link
Collaborator

I've updated the code according to the rules, thank you @DavdGao @zhijianma. I'm unsure about the best place to put the demo and UnitTest. Would you mind advising where they should go?

You can refer to https://github.com/modelscope/agentscope/pull/181/files to review the required changes. Demo, unitTest and documents are required.

@zhijianma
Copy link
Collaborator

zhijianma commented Jul 31, 2024

@Haijian06

Please review and improve your code, please refer to OpenAIChatWrapper .

  • Explicitly add stream parameter in the __init__ method.
  • Set stream to None as default value in the __call__ at L116.
  • stream is optional, please unify the method client.chat.completions.create in L136-L148.
  • Note that a streaming response does not have a model_dump method at line 157
  • Update the monitor for streaming mode at line 162.
  • Save a generator of content to the stream field of ModelResponse at line 167.
  • Handle the content and usage of the streaming response with care.
  • Complete the docstring accordingly.

Copy link
Collaborator

@DavdGao DavdGao left a comment

Choose a reason for hiding this comment

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

lgtm

@DavdGao DavdGao merged commit f40aaf4 into modelscope:main Aug 28, 2024
14 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