-
Notifications
You must be signed in to change notification settings - Fork 61
fix(bark): check the multinomial device query before sizing the launch #1391
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
Merged
chaofengw-nv
merged 1 commit into
NVIDIA:main
from
lukiod:fix/bark-multinomial-device-query
Sep 22, 2026
+228
−31
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| #include "families/bark/runtime/sparse_multinomial_kernel.h" | ||
|
|
||
| #include <algorithm> | ||
| #include <cstdint> | ||
| #include <stdexcept> | ||
| #include <string> | ||
|
|
||
| namespace trtmc { | ||
|
|
||
| namespace { | ||
|
|
||
| constexpr int kDistributionBlockSize = 256; | ||
|
|
||
| } // namespace | ||
|
|
||
| BarkTorchMultinomialExecutionPolicy bark_compute_torch_multinomial_execution_policy(int32_t numel) { | ||
| if (numel <= 0) { | ||
| return {}; | ||
| } | ||
|
|
||
| int device = 0; | ||
| const cudaError_t device_status = cudaGetDevice(&device); | ||
| if (device_status != cudaSuccess) { | ||
| throw std::runtime_error("cudaGetDevice failed for the bark multinomial launch: " + | ||
| std::string(cudaGetErrorString(device_status))); | ||
| } | ||
|
|
||
| cudaDeviceProp properties{}; | ||
| const cudaError_t properties_status = cudaGetDeviceProperties(&properties, device); | ||
| if (properties_status != cudaSuccess) { | ||
| throw std::runtime_error( | ||
| "cudaGetDeviceProperties failed for the bark multinomial launch: " + | ||
| std::string(cudaGetErrorString(properties_status))); | ||
| } | ||
|
|
||
| const uint32_t blocks_per_sm = | ||
| static_cast<uint32_t>(properties.maxThreadsPerMultiProcessor / kDistributionBlockSize); | ||
| const uint32_t grid = | ||
| std::min(static_cast<uint32_t>(properties.multiProcessorCount) * blocks_per_sm, | ||
| static_cast<uint32_t>((static_cast<uint64_t>(numel) + kDistributionBlockSize - 1) / | ||
| kDistributionBlockSize)); | ||
| const uint64_t total_threads = static_cast<uint64_t>(grid) * kDistributionBlockSize; | ||
| // A query can succeed and still report no usable occupancy. | ||
| if (total_threads == 0) { | ||
| throw std::runtime_error("bark multinomial launch policy computed no threads"); | ||
| } | ||
|
|
||
| const uint64_t counter_offset = | ||
| ((static_cast<uint64_t>(numel) - 1) / (total_threads * kGeneratorOffsetsPerCurandCall) + | ||
| 1) * | ||
| kGeneratorOffsetsPerCurandCall; | ||
|
|
||
| BarkTorchMultinomialExecutionPolicy policy; | ||
| policy.total_threads = static_cast<int32_t>(total_threads); | ||
| policy.counter_offset = counter_offset; | ||
| return policy; | ||
| } | ||
|
|
||
| } // namespace trtmc | ||
141 changes: 141 additions & 0 deletions
141
families/bark/tests/cpp/test_bark_multinomial_policy.cpp
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,141 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| // Compiles the host multinomial policy against CPU CUDA stubs, so a failed | ||
| // device query can be injected without a GPU or cudart. Verifies that a failed | ||
| // query throws instead of sizing a launch from an empty property block, and | ||
| // that a healthy device keeps the thread count and generator offset it had. | ||
|
|
||
| #include "families/bark/runtime/sparse_multinomial_kernel.h" | ||
|
|
||
| #include <cstdint> | ||
| #include <cstdio> | ||
| #include <cuda_runtime.h> | ||
| #include <stdexcept> | ||
| #include <string> | ||
|
|
||
| namespace { | ||
|
|
||
| cudaError_t g_device_status = cudaSuccess; | ||
| cudaError_t g_properties_status = cudaSuccess; | ||
| cudaDeviceProp g_properties{}; | ||
| int g_device_queries = 0; | ||
| int g_property_queries = 0; | ||
| int g_failures = 0; | ||
|
|
||
| void check(bool condition, const char* what) { | ||
| if (!condition) { | ||
| std::fprintf(stderr, "FAIL: %s\n", what); | ||
| ++g_failures; | ||
| } | ||
| } | ||
|
|
||
| void reset_stubs() { | ||
| g_device_status = cudaSuccess; | ||
| g_properties_status = cudaSuccess; | ||
| g_properties = cudaDeviceProp{}; | ||
| g_device_queries = 0; | ||
| g_property_queries = 0; | ||
| } | ||
|
|
||
| // Returns true when the call threw, and checks that the message names the | ||
| // operation that actually failed. | ||
| bool throws_naming(const char* expected, const char* what) { | ||
| try { | ||
| trtmc::bark_compute_torch_multinomial_execution_policy(1024); | ||
| } catch (const std::runtime_error& error) { | ||
| check(std::string(error.what()).find(expected) != std::string::npos, what); | ||
| return true; | ||
| } | ||
| check(false, what); | ||
| return false; | ||
| } | ||
|
|
||
| } // namespace | ||
|
|
||
| extern "C" { | ||
|
|
||
| cudaError_t cudaGetDevice(int* device) { | ||
| ++g_device_queries; | ||
| if (g_device_status != cudaSuccess) { | ||
| return g_device_status; | ||
| } | ||
| *device = 0; | ||
| return cudaSuccess; | ||
| } | ||
|
|
||
| cudaError_t cudaGetDeviceProperties(cudaDeviceProp* properties, int device) { | ||
| (void)device; | ||
| ++g_property_queries; | ||
| if (g_properties_status != cudaSuccess) { | ||
| return g_properties_status; | ||
| } | ||
| *properties = g_properties; | ||
| return cudaSuccess; | ||
| } | ||
|
|
||
| const char* cudaGetErrorString(cudaError_t error) { | ||
| return error == cudaErrorInsufficientDriver ? "insufficient driver" : "invalid device ordinal"; | ||
| } | ||
|
|
||
| } // extern "C" | ||
|
|
||
| int main() { | ||
| // A healthy device keeps the values the sampler divides by. The grid is | ||
| // capped by resident blocks (108 * 6 = 648) rather than by the request. | ||
| reset_stubs(); | ||
| g_properties.multiProcessorCount = 108; | ||
| g_properties.maxThreadsPerMultiProcessor = 1536; | ||
| const trtmc::BarkTorchMultinomialExecutionPolicy policy = | ||
| trtmc::bark_compute_torch_multinomial_execution_policy(1000000); | ||
| check(policy.total_threads == 165888, "a 108-SM device should keep its resident thread count"); | ||
| check(policy.counter_offset == 8, "a 108-SM device should keep its generator offset"); | ||
|
|
||
| // A request smaller than one resident grid is capped by the request itself. | ||
| reset_stubs(); | ||
| g_properties.multiProcessorCount = 108; | ||
| g_properties.maxThreadsPerMultiProcessor = 1536; | ||
| const trtmc::BarkTorchMultinomialExecutionPolicy small = | ||
| trtmc::bark_compute_torch_multinomial_execution_policy(256); | ||
| check(small.total_threads == 256, "a one-block request should keep one block of threads"); | ||
| check(small.counter_offset == 4, "a one-block request should keep its generator offset"); | ||
|
|
||
| // An empty request needs no device query at all. | ||
| reset_stubs(); | ||
| const trtmc::BarkTorchMultinomialExecutionPolicy empty = | ||
| trtmc::bark_compute_torch_multinomial_execution_policy(0); | ||
| check(empty.total_threads == 0, "an empty request should have no threads"); | ||
| check(empty.counter_offset == 0, "an empty request should have no offset"); | ||
| check(g_device_queries == 0, "an empty request should not query the device"); | ||
|
|
||
| // A device lookup that fails must throw, and must not go on to query the | ||
| // properties of a device it never resolved. | ||
| reset_stubs(); | ||
| g_device_status = cudaErrorInsufficientDriver; | ||
| check(throws_naming("cudaGetDevice failed", "a failed device lookup should throw naming it"), | ||
| "a failed device lookup should throw"); | ||
| check(g_property_queries == 0, "a failed device lookup should not query the device properties"); | ||
|
|
||
| // A property lookup that fails must throw naming its own error string. | ||
| reset_stubs(); | ||
| g_properties_status = cudaErrorInvalidDevice; | ||
| check(throws_naming("invalid device ordinal", | ||
| "a failed property lookup should report the CUDA error"), | ||
| "a failed property lookup should throw"); | ||
|
|
||
| // The reported fault: a query that succeeds but reports no usable occupancy | ||
| // used to divide by a zero thread count. | ||
| reset_stubs(); | ||
| check(throws_naming("computed no threads", | ||
| "a zeroed property block should throw instead of dividing by zero"), | ||
| "a zeroed property block should throw"); | ||
|
|
||
| if (g_failures != 0) { | ||
| std::fprintf(stderr, "%d check(s) failed\n", g_failures); | ||
| return 1; | ||
| } | ||
| std::printf("bark multinomial policy device-query checks passed\n"); | ||
| return 0; | ||
| } |
Oops, something went wrong.
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.
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.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Report CUDA error names instead of descriptions.
These branches call
cudaGetErrorString. They report text such asinsufficient driver, not names such ascudaErrorInsufficientDriver.Use
cudaGetErrorNameto meet the PR objective. Update the CPU stub and assertions infamilies/bark/tests/cpp/test_bark_multinomial_policy.cppaccordingly.Proposed fix
Also applies to: 36-38
🤖 Prompt for AI Agents