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.

Sign up for GitHub

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

Jump to bottom

Remove deprecated arguments from API and clarify model_name and chat_template_name #1931

Merged
merged 42 commits into from
Aug 8, 2024
Merged

Remove deprecated arguments from API and clarify model_name and chat_template_name #1931

merged 42 commits into from
Aug 8, 2024

Conversation

lvhan028
Copy link
Collaborator

@lvhan028 lvhan028 commented Jul 5, 2024 •
edited
Loading

  • remove get_hf_config_content. Use get_model_arch instead
  • remove deprecated argument such as tp, meta_instruction, cap and so on
  • remove deprecated functions in CLI, such as kv_qparams
  • tell "model_name" and "chat_template_name" apart. "model_name" means the name of the served model, which is used by the api_server "v1/models". "chat_template_name" means the name of the chat template
  • "--chat-template" can be a builtin chat template name of a customized json file

@lvhan028 lvhan028 removed the WIP label Jul 23, 2024
AllentDan
AllentDan reviewed Aug 2, 2024
View reviewed changes
lmdeploy/cli/utils.py
Comment on lines -229 to -234
return parser.add_argument(
'--cap',
type=str,
default='chat',
choices=['completion', 'infilling', 'chat', 'python'],
help='The capability of a model. '
Copy link
Collaborator

Choose a reason for hiding this comment

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

May update https://github.com/InternLM/lmdeploy/blob/main/autotest/utils/run_client_chat.py#L32

Copy link
Collaborator Author

@lvhan028 lvhan028 Aug 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@zhulinJulia24 I remove --cap from the CLI chat and use --chat-template <a json file> instead.
Could you help fix the test cases?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@zhulinJulia24 I remove --cap from the CLI chat and use --chat-template <a json file> instead. Could you help fix the test cases?

already fixed in pr #2183

irexyc
irexyc reviewed Aug 7, 2024
View reviewed changes
lmdeploy/turbomind/turbomind.py Outdated Show resolved Hide resolved
lmdeploy/turbomind/deploy/converter.py Show resolved Hide resolved
@lvhan028 lvhan028 mentioned this pull request Aug 7, 2024
Merged
@RunningLeon RunningLeon mentioned this pull request Aug 8, 2024
Merged
RunningLeon
RunningLeon approved these changes Aug 8, 2024
View reviewed changes
Copy link
Collaborator

@RunningLeon RunningLeon left a comment

Choose a reason for hiding this comment

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

LGTM

AllentDan
AllentDan approved these changes Aug 8, 2024
View reviewed changes
Copy link
Collaborator

@AllentDan AllentDan left a comment

Choose a reason for hiding this comment

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

LGTM

@lvhan028 lvhan028 merged commit fb6c5a1 into InternLM:main Aug 8, 2024
5 checks passed
@lzhangzz lzhangzz mentioned this pull request Aug 14, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhulinJulia24 zhulinJulia24 zhulinJulia24 left review comments

@irexyc irexyc irexyc left review comments

@AllentDan AllentDan AllentDan approved these changes

@RunningLeon RunningLeon RunningLeon approved these changes

Assignees
No one assigned
Labels
BC-breaking improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@lvhan028 @zhulinJulia24 @irexyc @RunningLeon @AllentDan