feat: Make llama.cpp Gnu OpenMP dependency optional #1331
Conversation
Do not include by default as it needs `libgomp1` at runtime. Add a feature to enable it at build time.
grahamking
requested review from
a team,
GuanLuo,
biswapanda,
kkranen,
oandreeva-nv,
paulhendricks,
rmccorm4,
ryanolson and
tmonty12
as code owners
WalkthroughThe changes introduce a new optional "openmp" feature flag for both the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Cargo
participant dynamo-run
participant dynamo-engine-llamacpp
participant llama-cpp-2
User->>Cargo: cargo build --features cuda,openmp -p dynamo-run
Cargo->>dynamo-run: Enable features: cuda, openmp
dynamo-run->>dynamo-engine-llamacpp: Enable openmp feature
dynamo-engine-llamacpp->>llama-cpp-2: Enable openmp feature (no default features)
llama-cpp-2-->>dynamo-engine-llamacpp: Build with OpenMP support
dynamo-engine-llamacpp-->>dynamo-run: Build with OpenMP support
dynamo-run-->>User: Built binary with OpenMP enabled
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/guides/dynamo_run.md (1)
327-331: Refine OpenMP documentation and code fence
Adjust punctuation for readability and specify the fenced block language for markdown linting:-for GNU OpenMP support add the `openmp` feature. On Ubuntu this requires `libgomp1` (part of `build-essential`) at build and runtime. +For GNU OpenMP support, add the `openmp` feature. On Ubuntu, this requires `libgomp1` (part of `build-essential`) at build and runtime. -``` -cargo build --features cuda,openmp -p dynamo-run -``` +```shell +cargo build --features cuda,openmp -p dynamo-run +```🧰 Tools
🪛 LanguageTool
[uncategorized] ~327-~327: Possible missing comma found.
Context: ...ulkan -p dynamo-run ``` For GNU OpenMP support add theopenmpfeature. On Ubuntu thi...(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~327-~327: It is considered good style to insert a comma after introductory phrases with dates or proper nouns.
Context: ...MP support add theopenmpfeature. On Ubuntu this requireslibgomp1(part of `buil...(IN_NNP_COMMA)
🪛 markdownlint-cli2 (0.17.2)
329-329: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/guides/dynamo_run.md(1 hunks)launch/dynamo-run/Cargo.toml(1 hunks)lib/engines/llamacpp/Cargo.toml(2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/guides/dynamo_run.md
[uncategorized] ~327-~327: Possible missing comma found.
Context: ...ulkan -p dynamo-run ``` For GNU OpenMP support add the openmp feature. On Ubuntu thi...
(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~327-~327: It is considered good style to insert a comma after introductory phrases with dates or proper nouns.
Context: ...MP support add the openmp feature. On Ubuntu this requires libgomp1 (part of `buil...
(IN_NNP_COMMA)
🪛 markdownlint-cli2 (0.17.2)
docs/guides/dynamo_run.md
329-329: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Mirror Repository to GitLab
- GitHub Check: pre-merge-rust (lib/runtime/examples)
- GitHub Check: pre-merge-rust (.)
- GitHub Check: Build and Test - vllm
- GitHub Check: pre-merge-rust (lib/bindings/python)
🔇 Additional comments (3)
lib/engines/llamacpp/Cargo.toml (2)
32-32: Introduce optional OpenMP feature for llama-cpp-2
Theopenmpfeature properly extends the existing[features]array, aligning withcuda,metal, andvulkan. This makes OpenMP support opt-in without altering defaults.
42-44: Disable default features for llama-cpp-2
Settingdefault-features = falseprevents inadvertent inclusion of OpenMP or other defaults, pushing all configuration to explicit flags. The inline comment clearly documents thelibgomp1requirement.launch/dynamo-run/Cargo.toml (1)
24-24: Addopenmpfeature to dynamo-run
Wiringopenmp = ["dynamo-engine-llamacpp/openmp"]into the CLI’s[features]section correctly propagates the engine’s OpenMP support. Defaults remain minimal.
Do not include by default as it needs libgomp1 at runtime. Add a feature to enable it at build time.
Do not include by default as it needs
libgomp1at runtime. Add a feature to enable it at build time.Summary by CodeRabbit
Documentation
New Features
Chores