-
Notifications
You must be signed in to change notification settings - Fork 263
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 corner cases in LocalWindowService truncation and In-context Adapter #1449
Fix corner cases in LocalWindowService truncation and In-context Adapter #1449
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
@@ -222,7 +222,7 @@ def construct_example_prompt(self, instance: Instance, include_output: bool, ref | |||
Returns a single example of the prompt. `include_output` controls whether the gold output is included. | |||
""" | |||
# Input | |||
result: str = self.adapter_spec.input_prefix + instance.input.text + self.adapter_spec.input_suffix | |||
result: str = self.adapter_spec.input_prefix + str(instance.input.text) + self.adapter_spec.input_suffix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
str(None)
is 'None'
, which is probably not what you want. If you want the empty string in the case of None
, you could use (instance.input.text or "")
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revised
# in https://github.com/stanford-crfm/helm/issues/1448). | ||
# Truncate by removing character by character until the prompt fits within the context window. | ||
while not self.fits_within_context_window(result, expected_completion_token_length): | ||
result = result[:-1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naively truncating characters from the right until it fits the context window sounds reasonable to me.
cc @teetone @percyliang FYI for the change in window service behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think truncation in the middle may also work (This avoid truncating "Answer: "
). Another approach is to use LLM itself to extract a shorter summary of the context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I revised it this way to align with the other window services.
instance.input.text
can beNone
, and may trigger str + None error. We simply wrapped it withstr()