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

fix t5 tokenizer and prompt token failures #1966

Merged
merged 1 commit into from
May 28, 2024

Conversation

rohithkrn
Copy link
Contributor

  1. In lmi-dist, t5 model impl is different from other models and preprocessor's tokenizer attribute contains tokenizer directly instead of TokenizerGroup
  2. skip sending prompt token details for t5 as it does not populate those in request output. Current code errors out as request_output.prompt_token_ids is an integer of bos token and fails in the for loop enumeration.

@rohithkrn rohithkrn requested review from zachgk, frankfliu and a team as code owners May 24, 2024 00:54
@@ -47,6 +47,8 @@ def __init__(self, model_id_or_path: str, properties: dict, **kwargs):
:param properties (dict): other properties of the model, such as decoder strategy
"""
self.lmi_dist_config = LmiDistRbProperties(**properties)
self.model_type = getattr(kwargs.get("model_config", None),
"model_type", None)
Copy link
Contributor

Choose a reason for hiding this comment

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

QQ: I see we are populating model_config, Where do we populate "model_type"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

model_config here is hugging face config.json which has model_type populated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh misread it, Model_type is read from model_config. Cool

@@ -75,6 +75,9 @@ def update_request_cache_with_output(request_cache: OrderedDict,
if "prompt_tokens_details" not in request_cache[
request_id] and request_output.prompt_logprobs:
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious, request_output should not have prompt_logprobs as well right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

from what I can read, request_output.prompt_logprobs is set to [float("nan")] and seems like it's passing that check.

@rohithkrn rohithkrn merged commit 714fabf into deepjavalibrary:master May 28, 2024
8 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.

None yet

3 participants