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

Add log input text when using openai chat api #2058

Closed
wants to merge 5 commits into from

Conversation

ccjincong
Copy link

Motivation

Support the feature #1608

Modifications

  1. encode the id to get the text
  2. delete an error check(not sure if this change will have any impact.)

Checklist

  • Format your code according to the Contributor Guide.
  • Add unit tests as outlined in the Contributor Guide.
  • Update documentation as needed, including docstrings or example tutorials.

@ccjincong ccjincong requested a review from merrymercy November 19, 2024 01:36
if (self.text is None and self.input_ids is None) or (
self.text is not None and self.input_ids is not None
):
if self.text is None and self.input_ids is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not think you can change this. Only one of them can be provided. Otherwise, it is ambiguous.

Copy link
Author

@ccjincong ccjincong Nov 24, 2024

Choose a reason for hiding this comment

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

Does that mean in order to support this feature, I can only provide text?
But I think it conflicts with https://github.com/sgl-project/sglang/blob/fa271613809bc5d901c0e864c4f9b9d3d3a101bd/python/sglang/srt/managers/tokenizer_manager.py#L204C1-L208C38.(The encoding here does not include add_special_tokens=False.)

Copy link
Contributor

Choose a reason for hiding this comment

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

you can move the logger to another place.

stop = request.stop
image_data = None
modalities = []

prompt_ids = tokenizer_manager.tokenizer.encode(prompt)
Copy link
Contributor

Choose a reason for hiding this comment

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

Add add_special_tokens=False

@merrymercy
Copy link
Contributor

Close this for now due to inactivity. Feel free to reopen if you have any updates.

@merrymercy merrymercy closed this Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants