From 29ae81fe83712bb74c00a4ce0515d031f1225c9b Mon Sep 17 00:00:00 2001 From: Alec <35311602+alec-flowers@users.noreply.github.com> Date: Thu, 8 Oct 2026 00:27:47 -0700 Subject: [PATCH] [Rust Frontend] Preserve explicit zero sampling values in gRPC requests (#60115) Signed-off-by: Alec Flowers --- rust/proto/inference.proto | 6 +-- rust/src/server/src/grpc/convert.rs | 63 +++++++++++++++++++++++------ rust/src/server/src/grpc/tests.rs | 34 ++++++++-------- 3 files changed, 71 insertions(+), 32 deletions(-) diff --git a/rust/proto/inference.proto b/rust/proto/inference.proto index 8189758212..79c1d0a147 100644 --- a/rust/proto/inference.proto +++ b/rust/proto/inference.proto @@ -75,9 +75,9 @@ message KVHintAction { message RandomSampling { uint32 num_sequences = 1; // "n", default (0) means 1 - uint32 top_k = 2; // 0 means default - float top_p = 3; // 0 means default - float min_p = 4; // 0 means default + optional uint32 top_k = 2; // Omitted means default; 0 disables top-k. + optional float top_p = 3; // Omitted means default; must be in (0, 1]. + optional float min_p = 4; // Omitted means default; 0 disables min-p. optional int64 seed = 5; } diff --git a/rust/src/server/src/grpc/convert.rs b/rust/src/server/src/grpc/convert.rs index 8f2c589f15..a409cc7f79 100644 --- a/rust/src/server/src/grpc/convert.rs +++ b/rust/src/server/src/grpc/convert.rs @@ -161,10 +161,7 @@ fn build_sampling_params( ..SamplingParams::default() }; - // RandomSampling: for every remaining sampling field the protobuf default (`0`) - // is treated as "unset" and leaves the resolved value to the lowering - // stage, which falls back to the model-provided default or a - // neutral/disabled value otherwise. + // Preserve explicit values; omitted fields inherit model defaults during lowering. if let Some(s) = sampling { // num_sequences (n > 1) is not supported yet by the TextLlm layer; the response // path also hardcodes SequenceOutput.index = 0, so accepting >1 would silently @@ -174,15 +171,9 @@ fn build_sampling_params( "num_sequences > 1 is not supported", )); } - if s.top_k != 0 { - params.top_k = Some(s.top_k); - } - if s.top_p != 0.0 { - params.top_p = Some(s.top_p); - } - if s.min_p != 0.0 { - params.min_p = Some(s.min_p); - } + params.top_k = s.top_k; + params.top_p = s.top_p; + params.min_p = s.min_p; params.seed = s.seed; } @@ -790,6 +781,52 @@ mod tests { } } + #[test] + fn grpc_sampling_preserves_explicit_values_and_model_defaults_through_lowering() { + for ((top_k, top_p, min_p), expected) in [ + ((None, None, None), (8, 0.9, 0.2)), + ((Some(0), Some(1.0), Some(0.0)), (0, 1.0, 0.0)), + ((Some(50), Some(0.8), Some(0.1)), (50, 0.8, 0.1)), + ] { + let request = pb::GenerateRequest { + temperature: Some(0.7), + sampling: Some(pb::RandomSampling { + top_k, + top_p, + min_p, + ..Default::default() + }), + ..base_request() + }; + let encoded = request.encode_to_vec(); + for stream in [false, true] { + let decoded = pb::GenerateRequest::decode(encoded.as_slice()).unwrap(); + let text = to_text_request(decoded, stream, &["test-model".to_string()]).unwrap(); + let engine = vllm_text::lower_text_request( + text, + vec![1], + SamplingHints { + default_top_k: Some(8), + default_top_p: Some(0.9), + default_min_p: Some(0.2), + ..Default::default() + }, + SamplingLimits { + max_model_len: 256, + max_logprobs: 20, + model_vocab_size: 512, + tokenizer_vocab_size: 512, + }, + &TestTokenizer::new(), + ) + .unwrap() + .generate_request; + let params = engine.sampling_params; + assert_eq!((params.top_k, params.top_p, params.min_p), expected); + } + } + } + #[test] fn bypass_prefix_cache_maps_to_skip_reading_prefix_cache() { let req = pb::GenerateRequest { diff --git a/rust/src/server/src/grpc/tests.rs b/rust/src/server/src/grpc/tests.rs index 631eb76b36..0ddbfb096e 100644 --- a/rust/src/server/src/grpc/tests.rs +++ b/rust/src/server/src/grpc/tests.rs @@ -1366,22 +1366,24 @@ async fn unary_generate_invalid_sampling_params_returns_invalid_argument() { ) .await; - let status = client - .generate(pb::GenerateRequest { - request_id: "test-invalid-sampling".to_string(), - model: "test-model".to_string(), - prompt: Some(pb::generate_request::Prompt::Text("hi".to_string())), - sampling: Some(pb::RandomSampling { - top_p: 2.0, + for top_p in [0.0, 2.0] { + let status = client + .generate(pb::GenerateRequest { + request_id: "test-invalid-sampling".to_string(), + model: "test-model".to_string(), + prompt: Some(pb::generate_request::Prompt::Text("hi".to_string())), + sampling: Some(pb::RandomSampling { + top_p: Some(top_p), + ..Default::default() + }), ..Default::default() - }), - ..Default::default() - }) - .await - .expect_err("should fail when top_p is out of range"); + }) + .await + .expect_err("should fail when top_p is out of range"); - assert_eq!(status.code(), tonic::Code::InvalidArgument); - assert!(status.message().contains("top_p")); + assert_eq!(status.code(), tonic::Code::InvalidArgument); + assert!(status.message().contains("top_p")); + } server_task.abort(); } @@ -1519,8 +1521,8 @@ async fn unary_generate_with_sampling_params() { prompt: Some(pb::generate_request::Prompt::Text("test".to_string())), temperature: Some(0.7), sampling: Some(pb::RandomSampling { - top_k: 50, - top_p: 0.9, + top_k: Some(50), + top_p: Some(0.9), seed: Some(42), ..Default::default() }),