Skip to content

Bug with instruct mode, somehow "forks" in background (Windows 11 - Powershell) and makes the shell unuseable #859

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

Closed
cmp-nct opened this issue Apr 9, 2023 · 5 comments
Labels

Comments

@cmp-nct
Copy link
Contributor

cmp-nct commented Apr 9, 2023

I can not provide a reproducible case but this has happened two times during various tests so there is something fishy.
main.exe was in Release mode, compiled in VS build tools and executed from Powershell in Win-11.

What happens is that CTRL+C is somehow accepted but the app doesn't stop - it pseudo-forks into background.
I am thrown back to the command prompt of the shell but the shell becomes extremely "laggy".
In one case I was able to issue commands but only every second key was accepted, so I could use the shell but had to repeat all keys twice.
main.exe waited in "prompt" mode I think, so it did not generate anything but waited (never received my input)

In the other case only the 'Enter' key was accepted (so the shell created a new line) but no other keys worked
In that case I also saw a generation happening, it was written into the shell.
So it was in generation mode but forked in the background.

In both cases the shell input was extremely laggy.
I've never seen that happening in Windows, it acted as if I had sent it into bash background using '&' plus extreme shell lag (for no reason, CPU was ok)

I closed the command shell which did not kill it's child main.exe
The solution in both cases was killing main.exe manually through the task manager.

I'm sorry I can't provide any steps to reproduce, in 99% of all cases it works fine. I didn't do anything different than usual.
Just wanted to make you aware about the bug, maybe the description helped.

@anzz1
Copy link
Contributor

anzz1 commented Apr 9, 2023

in regards to this #796

i think it should be

        // end of text token
-        if (!embd.empty() && embd.back() == llama_token_eos()) {
+        if (embd.empty() || embd.back() == llama_token_eos()) {

does that help?

@cmp-nct
Copy link
Contributor Author

cmp-nct commented Apr 9, 2023

hmm I don't think so.
Given it happens on CTRL+C (on abort) and the app actually moves into background like a fork, the problem is not that it ignores the end condition, the problem is that it somehow loses the parent process relation.
Something I've not seen happening in Windows before.

But I can not reproduce it easily, it happens randomly

@anzz1
Copy link
Contributor

anzz1 commented Apr 9, 2023

In interactive mode (instruct is interactive too) the first CTRL+C doesn't exit the app though if it's generating, it just stops the output and passes the control back to you.
https://github.com/ggerganov/llama.cpp/blob/aaf3b23debc1fe1a06733c8c6468fb84233cc44f/examples/main/main.cpp#L25-L37

It sounds like maybe that didn't happen though and you entered some weird loop state instead? That being said it's not impossible that using CTRL+C as a "break" condition instead of exit couldn't have some weird interaction.

@cmp-nct
Copy link
Contributor Author

cmp-nct commented Apr 9, 2023

In interactive mode (instruct is interactive too) the first CTRL+C doesn't exit the app though if it's generating, it just stops the output and passes the control back to you.

https://github.com/ggerganov/llama.cpp/blob/aaf3b23debc1fe1a06733c8c6468fb84233cc44f/examples/main/main.cpp#L25-L37

It sounds like maybe that didn't happen though and you entered some weird loop state instead? That being said it's not impossible that using CTRL+C as a "break" condition instead of exit couldn't have some weird interaction.

Simple ESC would be better and more intuitive for escaping. Maybe it's just that indeed.
One ESC to interrupt it, 2 ESC to end it. (that would require using a different handling of key input than signal handlers)

@github-actions github-actions bot added the stale label Mar 25, 2024
Copy link
Contributor

This issue was closed because it has been inactive for 14 days since being marked as stale.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants