-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix: Allow N-GPU Layers (NGL) to be set to 0 in llama.cpp #5907
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
Conversation
The `n_gpu_layers` (NGL) setting in the llama.cpp extension was incorrectly preventing users from disabling GPU layers by automatically defaulting to 100 when set to 0. This was caused by a condition that only pushed `cfg.n_gpu_layers` if it was greater than 0 (`cfg.n_gpu_layers > 0`). This commit updates the condition to `cfg.n_gpu_layers >= 0`, allowing 0 to be a valid and accepted value for NGL. This ensures that users can effectively disable GPU offloading when desired.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed everything up to 7cc408a in 48 seconds. Click for details.
- Reviewed
13
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. extensions/llamacpp-extension/src/index.ts:1185
- Draft comment:
Updated condition: using '>= 0' now properly accepts 0 as a valid value (disabling GPU layers) instead of defaulting to 100. This resolves #5899 if negative values are meant to fall back to 100. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, explaining a change in the code logic and its impact. It does not provide a suggestion, ask for confirmation, or request a test. It seems to be explaining the resolution of an issue, which is not within the guidelines for useful comments.
Workflow ID: wflow_kJ4lQLxAZLIl5EBZ
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Barecheck - Code coverage reportTotal: 37.41%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Changes requested ❌
Reviewed 23e2a32 in 2 minutes and 0 seconds. Click for details.
- Reviewed
15
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
0
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_XK3q5aBxBsLv5jc2
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
The
n_gpu_layers
(NGL) setting in the llama.cpp extension was incorrectly preventing users from disabling GPU layers by automatically defaulting to 100 when set to 0.This was caused by a condition that only pushed
cfg.n_gpu_layers
if it was greater than 0 (cfg.n_gpu_layers > 0
).This commit updates the condition to
cfg.n_gpu_layers >= 0
, allowing 0 to be a valid and accepted value for NGL. This ensures that users can effectively disable GPU offloading when desired.Fixes Issues
Important
Fixes issue in
index.ts
allowingn_gpu_layers
to be set to 0, enabling GPU layer disabling.index.ts
where settingn_gpu_layers
to 0 defaulted to 100, preventing GPU layer disabling.cfg.n_gpu_layers >= 0
to allow 0 as a valid value.This description was created by
for 23e2a32. You can customize this summary. It will automatically update as commits are pushed.