Repository navigation
Conversation
Avoid enabling unsupported sampling when callers leave sampling policy unset. Keep explicit sampling requests and other models' existing defaults unchanged. Files changed: - sdk_v2/cpp/src/inferencing/generative/chat/search_options.cc - sdk_v2/cpp/test/internal_api/chat/search_options_test.cc Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3b7a6cfe-bac8-4d81-a588-6eb681fdf36d
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified for the change preserving Nemotron Bolt’s configured greedy-search default.
No open issues 🎉
What changed in this PR
Preserves Nemotron Bolt’s configured greedy-search default in the C++ SDK’s Generator path when temperature is omitted or neutral (1), avoiding an unintended switch to unsupported sampling.
Changes:
- Keeps explicit non-neutral temperature handling and other models’ sampling defaults unchanged.
- Adds three regression tests using the checked-in tiny CPU decoder, requiring no downloads or GPU.
| File | Description |
|---|---|
sdk_v2/cpp/test/internal_api/chat/search_options_test.cc |
Tests Bolt’s default policy, explicit sampling, and unchanged defaults for other models. |
sdk_v2/cpp/src/inferencing/generative/chat/search_options.cc |
Preserves nemotron_bolt’s configured sampling default when no caller policy is specified. |
Review effort: Balanced
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Nemotron Bolt's decoder supports greedy search, and its model configuration sets
do_sample=false. When a caller omits temperature, Foundry's Generator path nevertheless setsdo_sample=true, overriding that supported model default and causing generation to fail.Changes
Preserve the model's sampling default for
nemotron_boltwhen the caller leaves sampling policy unspecified. An omitted or neutraltemperature=1therefore no longer enables sampling. Explicit non-neutral temperatures are still forwarded and remain subject to GenAI validation; other models retain Foundry's established sampling default.Add three regression tests using the checked-in tiny CPU decoder: omitted/neutral temperature, explicit sampling, and unchanged defaults for other models. No downloads or GPU are required.
This is independent of #1296 and #1312: it does not change native tool protocols or prefill chunking. The nullable-schema issue introduced by the native projection is documented separately in this comment.
Validation
Fresh main-based builds, rather than only the combined Bolt integration branch:
Used the existing cached CI dependencies on Windows and custom runtime dependencies on Linux; no dependency pins changed.
From the corresponding test binary directory:
./foundry_local_tests --gtest_filter="ModelSamplingOptionsTest.*:SamplingPlanTest.*:SearchOptionsParsingTest.*:DynamicEngineChatTest.*"On Linux, the new default-policy test was also run before applying the production fix: it failed for both omitted and neutral temperature because
do_samplebecame true. After the fix, all three new tests and the related sampling/parser/Engine tests passed.Changed-range clang-format 20 and
git diff --checkpassed. No warning suppressions or test exclusions were added. The full model-dependent suite was not run.An initial Windows Debug attempt was blocked by the existing cpp-httplib C4702 diagnostic (unreachable fallback after a throw with exceptions enabled). The normal RelWithDebInfo configuration was used instead, without disabling warnings.