-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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
common : refactor cli arg parsing #7675
Conversation
f325608
to
9d44c4a
Compare
-ins and --instruct were moved in #7675 I have adjusted the README accordingly. There was no trace of --chatml in the README.
seems this broke some imatrix options, trying to run imatrix with -o or --output-file for an output file yields: error: unknown argument: -o error: unknown argument: --output-file though based on the changes you made my guess is that it was never proper, but invalid use of gpt_params_parse made it go through |
@ggerganov will --instruct be introduced in future again? That was the feature I found most useful in the main cli binary. Or are there other options that can achieve the same result? |
@ericcurtin look at |
@Green-Sky -i -if -r don't seem to work if looking to create a basic CLI Ollama/ChatGPT-like assistant... --instruct works great for this... |
I too am quite bothered by the removal of "-ins". Previously it used to be really interactive because you had a shell-like prompt ">" inviting you to type. You could distinguish user inputs from outputs in copy-pastes of the output. Now I cannot find an equivalent. I've tried --interactive-first (which is really much less convenient to type than -ins BTW) but there's no invite anymore. For me this is a significant functional regression which will force me to stick to tag b3086 for a while. I really don't understand why features are removed. I can understand that it's unintended breakage of course, it always happens to any of us, but if the breakage is intentional I don't understand. |
In addition the commit is huge (3800 lines of patch), it's impossible to analyze, too bad it was merged as a huge one and not in small pieces :-( |
I do kinda have the feature forked here: https://github.com/ericcurtin/podman-llm You can run like: podman-llm run granite But preferably the feature would be upstream in llama.cpp here... |
@ericcurtin and conversation mode does not meet your needs? (personally not using it)
|
Thank you @Green-Sky it does the job like before indeed. So in the end it's a breaking change caused by a command line argument name change. At least -ins and -cml should be supported transparently (even emit a deprecation warning saying "no longer supported, use -cnv"). The help is so long now that you have little chance of stumbling over the new syntax when looking for the old one (which I did before coming here). |
@Green-Sky it seems to work sort of, get stranger output with it: $ podman --root /home/curtine/.local/share/podman-llm/storage run --rm -it --security-opt=label=disable -v/home/curtine:/home/curtine -v/tmp:/tmp -v/home/curtine/.cache/huggingface/:/root/.cache/huggingface/ granite llama-main -m /root/granite-3b-code-instruct.Q4_K_M.gguf --log-disable -cnv
$ podman --root /home/curtine/.local/share/podman-llm/storage run --rm -it --security-opt=label=disable -v/home/curtine:/home/curtine -v/tmp:/tmp -v/home/curtine/.cache/huggingface/:/root/.cache/huggingface/ granite llama-main -m /root/granite-3b-code-instruct.Q4_K_M.gguf --log-disable --instruct
|
you can always use:
to debug what you get. |
@Green-Sky in commit d94c6e0 --conversation seems to be completely broken:
and even when I fixed it was no good. I had written a tool that was daemonless/serverless, and used the main binary like so:
There seems to be no way to match this behaviour now, which is quite frustrating. |
TODO
params.instruct
params.chatml
params.escape = true
by defaultparams.n_ctx = 0
by defaultserver
params ingpt_params
retrieval
params ingpt_params
passkey
params ingpt_params