Skip to content

Conversation

@shirayu
Copy link
Contributor

@shirayu shirayu commented Feb 24, 2024

Added --eot_line option.
This allows multi lines input.

For example, we can use the following prompt with --eot_line EOT .

Please follow the instructions.

1. aaa
2. bbb
3. ccc
EOT

@shirayu shirayu changed the title Add Add --eot_line option Feb 24, 2024
@austinvhuang austinvhuang changed the base branch from main to dev February 24, 2024 17:29
@austinvhuang
Copy link
Contributor

There's definitely a need for this. If you notice in the shell demo there's this tr '\n' ' ' step to workaround the the issue.

It does change the function interface slightly and presence/absence of whitespace like "\n" can have subtle effects on quality, so the details require a bit of care.

Copy link
Contributor

@austinvhuang austinvhuang left a comment

Choose a reason for hiding this comment

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

This is really useful. let's bring this into dev - if folks have thoughts on the comments below or anything else feel free to chime in.

hwy::ThreadPool& inner_pool, const InferenceArgs& args,
int verbosity, const gcpp::AcceptFunc& accept_token) {
int verbosity, const gcpp::AcceptFunc& accept_token,
std::string &eot_line
Copy link
Contributor

Choose a reason for hiding this comment

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

can eot_line be const?

"estimate of "
"how many concurrent threads are supported.",
2);
visitor(eot_line, "eot_line", std::string(""),
Copy link
Contributor

Choose a reason for hiding this comment

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

@jan-wassenberg the way the CLI arg parsing works, are there certain characters that would be inexpressible (eg if you wanted some other whitespace like '\t' or ' '?

Another consideration to think about is, is end-of-prompt best specified as:

  • a string?
  • a character?
  • a token ID?

I wonder if either a character or token ID might be preferable to string (and then '\n' becomes a special case), but open to having other perspectives chime in here.

OTOH the nice thing about using a line string is that:

  • it can easily be entered in interactively (not always the case for a rare token/character)
  • if you need to allow for the entire vocabulary, you can effectively use a unique-by-design string as a quasi-out-of-vocabulary sentinel token.

Copy link
Member

Choose a reason for hiding this comment

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

I suppose we can post-process the string to implement escaping, then we can express anything, right?

if (line == eot_line) {
break;
}
prompt_string += line + "\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this makes inputs that don't end in "\n" inexpressible. This might be okay, but would be good to better understand the implications for quality.

@austinvhuang austinvhuang added the copybara-import Trigger Copybara for merging pull requests label Feb 27, 2024
@copybara-service copybara-service bot merged commit 1a1dd90 into google:dev Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

copybara-import Trigger Copybara for merging pull requests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants