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

Don't force immediate interactive without -i #354

Merged
merged 2 commits into from
Mar 22, 2023

Conversation

tjohnman
Copy link
Contributor

@tjohnman tjohnman commented Mar 21, 2023

Sometimes we might want to use a reverse prompt but we want to let the model generate tokens right after the initial prompt. So we don't force user input mode if the -i flag wasn't specified and instead let it run until we encounter the reverse prompt.

This gives use some more flexibility, since it doesn't force the user to enter a newline if they want to let the model generate text right after the initial prompt and only be asked for input if the reverse prompt is encountered.

@gjmulder gjmulder added the enhancement New feature or request label Mar 21, 2023
@blackhole89
Copy link
Contributor

blackhole89 commented Mar 21, 2023

I think it's good to not force interactive mode immediately (in fact that was how it worked when I first made the patch, but the logic seems to have changed at some point), but in this combination the flags seem to be rendered a bit misleading.

What I conceived of in the beginning:

  1. No -i or -r PROMPT: Classical, non-interactive generate-only mode.
  2. -i: Interactive mode, in which the user may add additional input during generation by pressing Ctrl+C to seize control.
  3. --interactive-first: As -i, plus prompt for user input immediately after the initial prompt is emitted.
  4. -r PROMPT: As -i, plus look for reverse prompt PROMPT, so control passes to the user whenever it is encountered.

The scenario you are proposing:

  1. No -i or -r PROMPT: Classical, non-interactive generate-only mode.
  2. Not available: interactive mode, where further user input is prompted after Ctrl+C only.
  3. -i: Interactive mode, plus prompt for user input immediately after the initial prompt is emitted.
  4. -r PROMPT: Interactive mode, plus look for reverse prompt PROMPT.

I think (2) is a valid use case, and moreover it is confusing that the "interactive mode" flag -i actually takes you into the "interactive + input in the beginning" mode while in order to get into any sort of interactive mode where you do not want to start out submitting input, you have to specify a reverse prompt with -r and not submit -i (and the natural UX for attaining 2 is -r RANDOM_STRING_THAT_NEVER_WILL_COME_UP).

Rather than loading initially_interacting from params.is_interactive, I would therefore suggest (re)introducing a dedicated parameter corresponding to it.

@tjohnman
Copy link
Contributor Author

tjohnman commented Mar 21, 2023

@blackhole89 I agree 100% with you that the first scenario is the most intuitive and useful (I'll do the changes). I did not remove --interactive-first (but I do remember seeing it in a previous build; no idea what happened to it).

@blackhole89
Copy link
Contributor

@tjohnman Thanks! Wasn't meaning to imply you had anything to do with the removal - development has been moving quickly and chaotically, it probably just fell on the wayside in some refactoring along the way.

Johnman added 2 commits March 21, 2023 18:21
Sometimes we might want to use a reverse prompt but we want to let the
model generate tokens right after the initial prompt. So we don't force
user input mode if the -i flag wasn't specified and instead let it run
until we encounter the reverse prompt.

This gives use some more flexibility, since it doesn't force the user to
enter a newline if they want to let the model generate text right after
the initial prompt and only be asked for input if the reverse prompt is
encountered.

The `--interactive-first` flag is reintroduced to force the old
behavior. `-r` behaves like `-i` plus introduces a reverse prompt (it
can be specified more than once).
@ggerganov ggerganov merged commit 305ba6f into ggml-org:master Mar 22, 2023
@tjohnman tjohnman deleted the no-forced-interactive-start branch March 22, 2023 17:45
@Green-Sky
Copy link
Collaborator

this kind of broke instruction mode. this change needs to be only for --interactive not for --instruct

@@ -1032,7 +1036,7 @@ int main(int argc, char ** argv) {
#endif
" - Press Return to return control to LLaMa.\n"
" - If you want to submit another line, end your input in '\\'.\n\n");
is_interacting = true;
is_interacting = params.interactive_start;
Copy link
Collaborator

@Green-Sky Green-Sky Mar 22, 2023

Choose a reason for hiding this comment

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

params.interactive_start || params.instruct

tjohnman pushed a commit to tjohnman/llama.cpp that referenced this pull request Mar 22, 2023
Green-Sky pushed a commit that referenced this pull request Mar 23, 2023
Co-authored-by: Johnman <tjohnman@github>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants