Skip to content

common : refactor arg parser#9308

Merged
ngxson merged 23 commits into
ggml-org:masterfrom
ngxson:xsn/argparser_v3
Sep 7, 2024
Merged

common : refactor arg parser#9308
ngxson merged 23 commits into
ggml-org:masterfrom
ngxson:xsn/argparser_v3

Conversation

@ngxson

@ngxson ngxson commented Sep 4, 2024

Copy link
Copy Markdown
Collaborator

TL;DR for breaking change

This PR has only some small breaking changes to the environment variable system introduced in #9105 :

  • To disable continuous batching, add LLAMA_ARG_NO_CONT_BATCHING=1 (instead of LLAMA_ARG_CONT_BATCHING=0)
  • To disable slots endpoint, add LLAMA_ARG_NO_ENDPOINT_SLOTS=1 (instead of LLAMA_ARG_ENDPOINT_SLOTS=0)
  • If both command line argument and environment variable are both set for the same param, the argument will take precedence over env var.
    In such case, you will also see a warning, for example: warn: LLAMA_ARG_CTX_SIZE environment variable is set, but will be overwritten by command line argument -c

In this PR

The goals of this PR are:

  • Refactor & make the arg parser code more intuitive, tailored to llama.cpp's usage
  • Ability to auto-generate documentation (markdown content) from code
  • Better support for multiple examples (plus, one arg used by multiple examples or different purposes)
  • Unifying env variable & arguments logic into one place

To generate markdown, run: make llama-export-docs
Output file will be in the format of autogen-{EXAMPLE_NAME}.md

TODO:

  • migrate all args to this new format
  • migrate env variables
  • add some ctests
  • add binary target to export markdown
  • update exiting markdown docs

List of removed args (this is not breaking change, since no where in the code base handles these args):

--priority
--priority-batch
--priority-draft
-Cbd
--priority-batch-draft

@ngxson

ngxson commented Sep 4, 2024

Copy link
Copy Markdown
Collaborator Author

@ggerganov Before proceeding further, I would like to ask for you opinion about this subject. Do you think this is a good way to have code-as-documentations? And if so, do you have any idea to add to this? Thank you.

@ggerganov ggerganov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems ok to me 👍

Comment thread common/common.h Outdated
@github-actions github-actions Bot added the server label Sep 5, 2024
@github-actions github-actions Bot added the testing Everything test related label Sep 5, 2024
@ngxson
ngxson marked this pull request as ready for review September 5, 2024 18:23
@ngxson ngxson changed the title common : refactor arg parser (WIP) common : refactor arg parser Sep 5, 2024
@ngxson ngxson added the breaking change Changes that break ABIs, APIs, file formats, or other forms of backwards compatibility. label Sep 5, 2024
@ngxson

ngxson commented Sep 5, 2024

Copy link
Copy Markdown
Collaborator Author

@ggerganov Thank you for the initial review. This PR is now ready.

Here is a quick recap on what I've done:

  • All CLI args & env var are migrated to the new format
  • tests/test-arg-parser is added to test this new system
  • llama-export-docs target is added, which can export list of arguments to markdown table (instead of markdown list in my initial demo). Here is an example for server docs

@ngxson
ngxson requested a review from ggerganov September 5, 2024 19:30
Comment thread tests/test-arg-parser.cpp Outdated
Comment thread tests/test-arg-parser.cpp Outdated
@ggerganov

ggerganov commented Sep 7, 2024

Copy link
Copy Markdown
Member

Functionality-wise, this is great. However the build time of libcommon increases on my machine from ~3s to ~12s:

ccache -C && touch ../common/common.cpp && time make -j common

Should we try to reduce it in some way? I suppose the culprit is in the lambda handlers in gpt_params_parser_init.

@ngxson

ngxson commented Sep 7, 2024

Copy link
Copy Markdown
Collaborator Author

Quick question: because sparams is already included inside params, should we get rid of passing sparams as argument of handler function? (and replace it with params.sparams

Co-authored-by: slaren@users.noreply.github.com
@ngxson

ngxson commented Sep 7, 2024

Copy link
Copy Markdown
Collaborator Author

Alright, I change some std::string to const char * and it brought down the build time a little bit. Keep in mind that the 5.2s result above was with only one arg repeated 190 times.

The build time of the latest commit e625f5f :

$ make clean && time make -j common/common.o
make -j common/common.o  5.70s user 0.16s system 99% cpu 5.889 total

# versus master
# make -j common/common.o  5.48s user 0.16s system 99% cpu 5.674 total

So that's 0.2s slower compared to master. It can be further reduced if std::string help can be changed to const char *, but currently it's a quite tricky to have format(...) function to return const char *, so I think we can consider it later.

Comment thread examples/export-docs/export-docs.cpp
Comment thread examples/export-docs/export-docs.cpp
Comment thread common/common.h Outdated
Comment thread common/common.h

void gpt_params_parse_from_env(gpt_params & params);
void gpt_params_handle_model_default(gpt_params & params);
struct llama_arg {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should move the method implementations in the .cpp, to avoid building the same code in all examples.

Can also move all llama_arg related stuff in common/arg.h,.cpp. Can be in a follow-up PR

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Currently doing this will break gen-docs, since it reads the data directly from class member.

But yes I will do a follow up to split it into common/arg.h,.cpp

@ngxson
ngxson merged commit 1b9ae51 into ggml-org:master Sep 7, 2024
dsx1986 pushed a commit to dsx1986/llama.cpp that referenced this pull request Oct 29, 2024
* (wip) argparser v3

* migrated

* add test

* handle env

* fix linux build

* add export-docs example

* fix build (2)

* skip build test-arg-parser on windows

* update server docs

* bring back missing --alias

* bring back --n-predict

* clarify test-arg-parser

* small correction

* add comments

* fix args with 2 values

* refine example-specific args

* no more lamba capture

Co-authored-by: slaren@users.noreply.github.com

* params.sparams

* optimize more

* export-docs --> gen-docs
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 15, 2024
* (wip) argparser v3

* migrated

* add test

* handle env

* fix linux build

* add export-docs example

* fix build (2)

* skip build test-arg-parser on windows

* update server docs

* bring back missing --alias

* bring back --n-predict

* clarify test-arg-parser

* small correction

* add comments

* fix args with 2 values

* refine example-specific args

* no more lamba capture

Co-authored-by: slaren@users.noreply.github.com

* params.sparams

* optimize more

* export-docs --> gen-docs
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 18, 2024
* (wip) argparser v3

* migrated

* add test

* handle env

* fix linux build

* add export-docs example

* fix build (2)

* skip build test-arg-parser on windows

* update server docs

* bring back missing --alias

* bring back --n-predict

* clarify test-arg-parser

* small correction

* add comments

* fix args with 2 values

* refine example-specific args

* no more lamba capture

Co-authored-by: slaren@users.noreply.github.com

* params.sparams

* optimize more

* export-docs --> gen-docs
Seunghhon pushed a commit to Seunghhon/llama.cpp that referenced this pull request Apr 26, 2026
* (wip) argparser v3

* migrated

* add test

* handle env

* fix linux build

* add export-docs example

* fix build (2)

* skip build test-arg-parser on windows

* update server docs

* bring back missing --alias

* bring back --n-predict

* clarify test-arg-parser

* small correction

* add comments

* fix args with 2 values

* refine example-specific args

* no more lamba capture

Co-authored-by: slaren@users.noreply.github.com

* params.sparams

* optimize more

* export-docs --> gen-docs
ljubomirj pushed a commit to ljubomirj/llama.cpp that referenced this pull request May 6, 2026
* (wip) argparser v3

* migrated

* add test

* handle env

* fix linux build

* add export-docs example

* fix build (2)

* skip build test-arg-parser on windows

* update server docs

* bring back missing --alias

* bring back --n-predict

* clarify test-arg-parser

* small correction

* add comments

* fix args with 2 values

* refine example-specific args

* no more lamba capture

Co-authored-by: slaren@users.noreply.github.com

* params.sparams

* optimize more

* export-docs --> gen-docs
my-other-github-account pushed a commit to my-other-github-account/llama.cpp that referenced this pull request May 15, 2026
* (wip) argparser v3

* migrated

* add test

* handle env

* fix linux build

* add export-docs example

* fix build (2)

* skip build test-arg-parser on windows

* update server docs

* bring back missing --alias

* bring back --n-predict

* clarify test-arg-parser

* small correction

* add comments

* fix args with 2 values

* refine example-specific args

* no more lamba capture

Co-authored-by: slaren@users.noreply.github.com

* params.sparams

* optimize more

* export-docs --> gen-docs
my-other-github-account pushed a commit to my-other-github-account/llama.cpp that referenced this pull request May 15, 2026
* (wip) argparser v3

* migrated

* add test

* handle env

* fix linux build

* add export-docs example

* fix build (2)

* skip build test-arg-parser on windows

* update server docs

* bring back missing --alias

* bring back --n-predict

* clarify test-arg-parser

* small correction

* add comments

* fix args with 2 values

* refine example-specific args

* no more lamba capture

Co-authored-by: slaren@users.noreply.github.com

* params.sparams

* optimize more

* export-docs --> gen-docs
phibya pushed a commit to ziee-ai/llama.cpp that referenced this pull request May 29, 2026
* (wip) argparser v3

* migrated

* add test

* handle env

* fix linux build

* add export-docs example

* fix build (2)

* skip build test-arg-parser on windows

* update server docs

* bring back missing --alias

* bring back --n-predict

* clarify test-arg-parser

* small correction

* add comments

* fix args with 2 values

* refine example-specific args

* no more lamba capture

Co-authored-by: slaren@users.noreply.github.com

* params.sparams

* optimize more

* export-docs --> gen-docs
AlexiAlp pushed a commit to minghaop/llama.cpp that referenced this pull request Jun 2, 2026
* (wip) argparser v3

* migrated

* add test

* handle env

* fix linux build

* add export-docs example

* fix build (2)

* skip build test-arg-parser on windows

* update server docs

* bring back missing --alias

* bring back --n-predict

* clarify test-arg-parser

* small correction

* add comments

* fix args with 2 values

* refine example-specific args

* no more lamba capture

Co-authored-by: slaren@users.noreply.github.com

* params.sparams

* optimize more

* export-docs --> gen-docs
AlexiAlp pushed a commit to minghaop/llama.cpp that referenced this pull request Jun 2, 2026
* (wip) argparser v3

* migrated

* add test

* handle env

* fix linux build

* add export-docs example

* fix build (2)

* skip build test-arg-parser on windows

* update server docs

* bring back missing --alias

* bring back --n-predict

* clarify test-arg-parser

* small correction

* add comments

* fix args with 2 values

* refine example-specific args

* no more lamba capture

Co-authored-by: slaren@users.noreply.github.com

* params.sparams

* optimize more

* export-docs --> gen-docs
fukuro-kun pushed a commit to fukuro-kun/fukuro-llama-cpp-turboquant that referenced this pull request Jul 5, 2026
* (wip) argparser v3

* migrated

* add test

* handle env

* fix linux build

* add export-docs example

* fix build (2)

* skip build test-arg-parser on windows

* update server docs

* bring back missing --alias

* bring back --n-predict

* clarify test-arg-parser

* small correction

* add comments

* fix args with 2 values

* refine example-specific args

* no more lamba capture

Co-authored-by: slaren@users.noreply.github.com

* params.sparams

* optimize more

* export-docs --> gen-docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Changes that break ABIs, APIs, file formats, or other forms of backwards compatibility. examples server testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants