Skip to content

common : move arg parser code to arg.cpp#9388

Merged
ngxson merged 10 commits into
ggml-org:masterfrom
ngxson:xsn/arg_cpp
Sep 9, 2024
Merged

common : move arg parser code to arg.cpp#9388
ngxson merged 10 commits into
ggml-org:masterfrom
ngxson:xsn/arg_cpp

Conversation

@ngxson

@ngxson ngxson commented Sep 9, 2024

Copy link
Copy Markdown
Collaborator

Ref comment: #9308 (comment)

In this PR:

  • Move arg parser code to arg.cpp and arg.h
  • Add set_sparam() to mark a specific arg as "sampling param"
  • Re-categorized some args to the correct example (for example: llama-lookup and llama-parallel)

(Tested) Build time is not increased compared to master branch.


Comment thread common/arg.h Outdated
@github-actions github-actions Bot added testing Everything test related examples server labels Sep 9, 2024
Comment thread examples/batched-bench/batched-bench.cpp Outdated
@ngxson
ngxson merged commit bfe76d4 into ggml-org:master Sep 9, 2024
Comment thread common/arg.cpp
else {
throw std::invalid_argument("invalid value");
}
#ifndef GGML_USE_CUDA_SYCL_VULKAN

@m18coppola m18coppola Sep 10, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think this #ifndef is working as intended

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.

can you suggest a patch?

@m18coppola m18coppola Sep 10, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
#ifndef GGML_USE_CUDA_SYCL_VULKAN
#if defined(GGML_CUDA) || defined(GGML_SYCL) || defined(GGML_VULKAN)
#define GGML_USE_CUDA_SYCL_VULKAN
#endif
#ifndef GGML_USE_CUDA_SYCL_VULKAN

I'm not sure if the idiosyncrasy is to do this here or in the makefile. If it's decided to do this in the makefile, I wouldn't be confident on how to get it into the cmake build system.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Upon review, it looks like the macro is already in common.cpp. Maybe it needs to be moved to the header file?

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.

Yes we can move it here. However, the naming is quite misleading, so I'd suggest not to abstract it:

Suggested change
#ifndef GGML_USE_CUDA_SYCL_VULKAN
#if (!defined(GGML_USE_CUDA) && !defined(GGML_USE_SYCL)) && !defined(GGML_USE_VULKAN)

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.

Please have a look at the PR: #9411

@ngxson ngxson mentioned this pull request Sep 10, 2024
4 tasks
dsx1986 pushed a commit to dsx1986/llama.cpp that referenced this pull request Oct 29, 2024
* common : move arg parser to arg.cpp

* better categorize args

* add cmake

* missing climits

* missing cstdarg

* common : more explicit includes

* fix build

* refactor gpt_params_parse

* update server readme

* fix test

---------

Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 15, 2024
* common : move arg parser to arg.cpp

* better categorize args

* add cmake

* missing climits

* missing cstdarg

* common : more explicit includes

* fix build

* refactor gpt_params_parse

* update server readme

* fix test

---------

Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 18, 2024
* common : move arg parser to arg.cpp

* better categorize args

* add cmake

* missing climits

* missing cstdarg

* common : more explicit includes

* fix build

* refactor gpt_params_parse

* update server readme

* fix test

---------

Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
Seunghhon pushed a commit to Seunghhon/llama.cpp that referenced this pull request Apr 26, 2026
* common : move arg parser to arg.cpp

* better categorize args

* add cmake

* missing climits

* missing cstdarg

* common : more explicit includes

* fix build

* refactor gpt_params_parse

* update server readme

* fix test

---------

Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
ljubomirj pushed a commit to ljubomirj/llama.cpp that referenced this pull request May 6, 2026
* common : move arg parser to arg.cpp

* better categorize args

* add cmake

* missing climits

* missing cstdarg

* common : more explicit includes

* fix build

* refactor gpt_params_parse

* update server readme

* fix test

---------

Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
my-other-github-account pushed a commit to my-other-github-account/llama.cpp that referenced this pull request May 15, 2026
* common : move arg parser to arg.cpp

* better categorize args

* add cmake

* missing climits

* missing cstdarg

* common : more explicit includes

* fix build

* refactor gpt_params_parse

* update server readme

* fix test

---------

Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
my-other-github-account pushed a commit to my-other-github-account/llama.cpp that referenced this pull request May 15, 2026
* common : move arg parser to arg.cpp

* better categorize args

* add cmake

* missing climits

* missing cstdarg

* common : more explicit includes

* fix build

* refactor gpt_params_parse

* update server readme

* fix test

---------

Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
phibya pushed a commit to ziee-ai/llama.cpp that referenced this pull request May 29, 2026
* common : move arg parser to arg.cpp

* better categorize args

* add cmake

* missing climits

* missing cstdarg

* common : more explicit includes

* fix build

* refactor gpt_params_parse

* update server readme

* fix test

---------

Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
AlexiAlp pushed a commit to minghaop/llama.cpp that referenced this pull request Jun 2, 2026
* common : move arg parser to arg.cpp

* better categorize args

* add cmake

* missing climits

* missing cstdarg

* common : more explicit includes

* fix build

* refactor gpt_params_parse

* update server readme

* fix test

---------

Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
AlexiAlp pushed a commit to minghaop/llama.cpp that referenced this pull request Jun 2, 2026
* common : move arg parser to arg.cpp

* better categorize args

* add cmake

* missing climits

* missing cstdarg

* common : more explicit includes

* fix build

* refactor gpt_params_parse

* update server readme

* fix test

---------

Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
fukuro-kun pushed a commit to fukuro-kun/fukuro-llama-cpp-turboquant that referenced this pull request Jul 5, 2026
* common : move arg parser to arg.cpp

* better categorize args

* add cmake

* missing climits

* missing cstdarg

* common : more explicit includes

* fix build

* refactor gpt_params_parse

* update server readme

* fix test

---------

Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

examples server testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants