Add backend sampler for penalties sampler - #25262
Conversation
- Set default value for penalty_last_n based on model context if not specified. - Ensure penalty_last_n and n_prev are non-negative. - Update llama_sampler_penalties structure to inherit from llama_sampler_backend and add backend input handling for penalties. - Implement backend initialization and application logic for penalties, including frequency and presence adjustments.
- Introduced `accept_prompt` and `unique_prompt_tokens` functions to handle prompt acceptance and token uniqueness. - Implemented `compare_penalties_logits` to compare logits from backend and CPU samplers with penalties. - Added `test_backend_penalties_sampling` to validate backend penalties with various configurations. - Enhanced the test suite for better coverage of penalty handling in sampling.
… logits as -Inf and no longer generate NaN.
|
Hi @kmorennv, thanks for your contribution! Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:
Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below. |
|
I think that’s a bot error, since I only have a single open PR. |
|
(just for clarity's sake, this is just an informational message and it's not hard-enforced anywhere and since that's just a draft that doesn't really matter, but technically it's not an error) |
|
@pwilkin thanks , I filtered for this but only for open --> clear , it was not shown ... |
ORippler
left a comment
There was a problem hiding this comment.
Thanks for this nice feature addition!
From my side, I'm unsure if we should support [n_ctx, n_vocab]-case for top/min-p -> penalty sampler. Is this a commonly used sampler chain?
…d tests for invalid values
…lary-sized count tensor
danbev
left a comment
There was a problem hiding this comment.
I found the changes in test-backend-sampler.cpp somewhat hard to follow and think it could possibly be simplified. But I realize this might be subjective and since this PR has been open for a while we can perhaps followup and refactor this later if others feel the same.
ORippler
left a comment
There was a problem hiding this comment.
I found the changes in test-backend-sampler.cpp somewhat hard to follow and think it could possibly be simplified.
+1
| struct ggml_tensor * probs; | ||
| struct ggml_tensor * sampled; | ||
| struct ggml_tensor * candidates; | ||
| int64_t n_vocab; |
There was a problem hiding this comment.
The n_vocab should not be part of this struct. There is already precedent for passing the n_vocab explicitly during sampler construction when it is needed:
Lines 1471 to 1474 in 96278e3
Lines 1376 to 1381 in 96278e3
Either try to avoid it completely (the CPU version does not need it) or change the sampler init and pass it there.
| const int32_t n_ctx = cparams.n_ctx > 0 ? (int32_t) cparams.n_ctx : llama_model_n_ctx_train(model); | ||
| for (int i = 0; i < (int) cparams.n_seq_max; ++i) { | ||
| pimpl->samplers[i].reset(common_sampler_init(model, params.sampling)); | ||
| pimpl->samplers[i].reset(common_sampler_init(model, params.sampling, n_ctx)); | ||
| pimpl->samplers_seq_config[i] = { i, common_sampler_get(pimpl->samplers[i].get()) }; | ||
| } |
There was a problem hiding this comment.
This is not exactly correct because the llama_context is not yet created and we don't know the exact context that will be allocated. That's the reason similar logic was disabled above:
Lines 1288 to 1297 in 96278e3
In general, I think that supporting a parameter of -1 to mean the "context size" is not worth the added complexity and it's much simpler to have an upper limit for these type of parameters. For example, I don't think there is any reason to penalize in a window of more than 1024 tokens, so setting -1 could mean setting automatically setting it to 1024.
This matches how it is done for logit_bias and mirostat samplers, see #25262 (comment)
* sampling: enhance penalty handling in common_sampler_init - Set default value for penalty_last_n based on model context if not specified. - Ensure penalty_last_n and n_prev are non-negative. - Update llama_sampler_penalties structure to inherit from llama_sampler_backend and add backend input handling for penalties. - Implement backend initialization and application logic for penalties, including frequency and presence adjustments. * tests: add backend penalties sampling tests and utility functions - Introduced `accept_prompt` and `unique_prompt_tokens` functions to handle prompt acceptance and token uniqueness. - Implemented `compare_penalties_logits` to compare logits from backend and CPU samplers with penalties. - Added `test_backend_penalties_sampling` to validate backend penalties with various configurations. - Enhanced the test suite for better coverage of penalty handling in sampling. * sampling: add support for top-k penalties in backend sampling * sampling: add fix to ensure stable numerical results. Preserve masked logits as -Inf and no longer generate NaN. * sampling: enhance penalty comparison tests with masking penalties logic * add comments on padding * sampling: add comments on modifications * add the unit test to cover masked-out token as -INF * validate repeat penalty to ensure it is finite and greater than 0; add tests for invalid values * refactor: test functions to share logic and be less verbose * add test to cover case where previously penalized token is not part of candidates * remove comments * remove redundant penalty_last_n initialization and validation in common_sampler_init * add support for penalties in sampler chain with configurable positions * add validation for penalty parameters and enhance tests for non-finite values * add context parameter to common_sampler_init and set default for penalty_last_n * add llama_n_ctx parameter to common_sampler_init for improved sampler initialization * replace penalty_last_n x n_candidates comparison matrix with a vocabulary-sized count tensor * add tests for backend penalties sampling without filler entries , token_count.size() == n_active == n_max == 64 * add test for backend penalties sampling after top-p with large history window * remove as unused * add is_disabled method, tensor logits reshape, add rest review suggestions * clarify comment
…-org#26520) This matches how it is done for logit_bias and mirostat samplers, see ggml-org#25262 (comment)
This matches how it is done for logit_bias and mirostat samplers, see ggml-org/llama.cpp#25262 (comment)
* test new flash_attn test * rebase and fix to disable subgrou matrices when max_kv_tile == 0 * delete log output * Add i32 support to cpy and enables the all ops test * restore the non target ci tests * comment out of TODO of build-cpu.yml * fix format
* sampling: enhance penalty handling in common_sampler_init - Set default value for penalty_last_n based on model context if not specified. - Ensure penalty_last_n and n_prev are non-negative. - Update llama_sampler_penalties structure to inherit from llama_sampler_backend and add backend input handling for penalties. - Implement backend initialization and application logic for penalties, including frequency and presence adjustments. * tests: add backend penalties sampling tests and utility functions - Introduced `accept_prompt` and `unique_prompt_tokens` functions to handle prompt acceptance and token uniqueness. - Implemented `compare_penalties_logits` to compare logits from backend and CPU samplers with penalties. - Added `test_backend_penalties_sampling` to validate backend penalties with various configurations. - Enhanced the test suite for better coverage of penalty handling in sampling. * sampling: add support for top-k penalties in backend sampling * sampling: add fix to ensure stable numerical results. Preserve masked logits as -Inf and no longer generate NaN. * sampling: enhance penalty comparison tests with masking penalties logic * add comments on padding * sampling: add comments on modifications * add the unit test to cover masked-out token as -INF * validate repeat penalty to ensure it is finite and greater than 0; add tests for invalid values * refactor: test functions to share logic and be less verbose * add test to cover case where previously penalized token is not part of candidates * remove comments * remove redundant penalty_last_n initialization and validation in common_sampler_init * add support for penalties in sampler chain with configurable positions * add validation for penalty parameters and enhance tests for non-finite values * add context parameter to common_sampler_init and set default for penalty_last_n * add llama_n_ctx parameter to common_sampler_init for improved sampler initialization * replace penalty_last_n x n_candidates comparison matrix with a vocabulary-sized count tensor * add tests for backend penalties sampling without filler entries , token_count.size() == n_active == n_max == 64 * add test for backend penalties sampling after top-p with large history window * remove as unused * add is_disabled method, tensor logits reshape, add rest review suggestions * clarify comment
…-org#26520) This matches how it is done for logit_bias and mirostat samplers, see ggml-org#25262 (comment)
* sampling: enhance penalty handling in common_sampler_init - Set default value for penalty_last_n based on model context if not specified. - Ensure penalty_last_n and n_prev are non-negative. - Update llama_sampler_penalties structure to inherit from llama_sampler_backend and add backend input handling for penalties. - Implement backend initialization and application logic for penalties, including frequency and presence adjustments. * tests: add backend penalties sampling tests and utility functions - Introduced `accept_prompt` and `unique_prompt_tokens` functions to handle prompt acceptance and token uniqueness. - Implemented `compare_penalties_logits` to compare logits from backend and CPU samplers with penalties. - Added `test_backend_penalties_sampling` to validate backend penalties with various configurations. - Enhanced the test suite for better coverage of penalty handling in sampling. * sampling: add support for top-k penalties in backend sampling * sampling: add fix to ensure stable numerical results. Preserve masked logits as -Inf and no longer generate NaN. * sampling: enhance penalty comparison tests with masking penalties logic * add comments on padding * sampling: add comments on modifications * add the unit test to cover masked-out token as -INF * validate repeat penalty to ensure it is finite and greater than 0; add tests for invalid values * refactor: test functions to share logic and be less verbose * add test to cover case where previously penalized token is not part of candidates * remove comments * remove redundant penalty_last_n initialization and validation in common_sampler_init * add support for penalties in sampler chain with configurable positions * add validation for penalty parameters and enhance tests for non-finite values * add context parameter to common_sampler_init and set default for penalty_last_n * add llama_n_ctx parameter to common_sampler_init for improved sampler initialization * replace penalty_last_n x n_candidates comparison matrix with a vocabulary-sized count tensor * add tests for backend penalties sampling without filler entries , token_count.size() == n_active == n_max == 64 * add test for backend penalties sampling after top-p with large history window * remove as unused * add is_disabled method, tensor logits reshape, add rest review suggestions * clarify comment
…-org#26520) This matches how it is done for logit_bias and mirostat samplers, see ggml-org#25262 (comment)
…ml-org#26566) * test new flash_attn test * rebase and fix to disable subgrou matrices when max_kv_tile == 0 * delete log output * Add i32 support to cpy and enables the all ops test * restore the non target ci tests * comment out of TODO of build-cpu.yml * fix format
…ml-org#26566) * test new flash_attn test * rebase and fix to disable subgrou matrices when max_kv_tile == 0 * delete log output * Add i32 support to cpy and enables the all ops test * restore the non target ci tests * comment out of TODO of build-cpu.yml * fix format
…ml-org#26566) * test new flash_attn test * rebase and fix to disable subgrou matrices when max_kv_tile == 0 * delete log output * Add i32 support to cpy and enables the all ops test * restore the non target ci tests * comment out of TODO of build-cpu.yml * fix format
Overview & Motivation
This PR migrates penalties sampling (repeat, frequency, and presence) from the CPU to the GPU backend.
Performance Impact
Moving penalty sampling to the backend yields a noticeable boost in token generation speed:
gpt-oss-20b,Qwen3.6-35Bgpt-oss-20b,Qwen3.6-35BDetails performance
CTK - CudaToolKit
BS- backend-sampling
Command used to run benchmark:
./build/bin/llama-server -m /gguf/gpt-oss-20b-mxfp4.gguf --temp 1.0 --top-k 0 --top-p 1.0 --min-p 0.0 -dio --port 8033 -np 1 -b 4096 -ub 4096 --repeat-penalty 1.1 --presence-penalty 0 -bs
Core Implementation Steps
1. Backend Integration & Fallback
llama_sampler_penaltiesnow inherits fromllama_sampler_backend.ggmloperations, it falls back to the CPU to preserve compatibility.2. State Management & Clone Fix
prevring buffer andtoken_count) is still maintained on the CPU via the normalaccept()call, but the actual logit transformation is offloaded to the GPU.token_countare copied. Previously, an empty cloned token count delayed penalty applications.3. Sparse Graph Optimization
4. Support After Top-K / Top-P / Min-P
5. Testing
Requirements