Two defects every model family inherited: an unguarded device query and GPU tests that pass without running

[ruiling-smartbear]

Both of these came out of the review on #1158. Neither is specific to that family: the code is duplicated per family by design, so a single original decision is now present in 31 to 34 copies. I have not opened a PR for either yet, because the shape is worth agreeing on first.

1. sparse_multinomial_kernel.cu divides by a discarded device query

int device = 0;
cudaGetDevice(&device);                      // return value discarded
cudaDeviceProp props{};                      // zero-initialised
cudaGetDeviceProperties(&props, device);     // return value discarded

const uint32_t blocks_per_sm =
    static_cast<uint32_t>(props.maxThreadsPerMultiProcessor / kDistributionBlockSize);
const uint32_t grid = std::min(
    static_cast<uint32_t>(props.multiProcessorCount) * blocks_per_sm, ...);
const uint64_t total_threads = static_cast<uint64_t>(grid) * kDistributionBlockSize;
const uint64_t counter_offset =
    ((static_cast<uint64_t>(numel) - 1)
     / (total_threads * kGeneratorOffsetsPerCurandCall) + 1)   // divides by total_threads
    * kGeneratorOffsetsPerCurandCall;

A failed query leaves props as it was, so blocks_per_sm, grid and total_threads are all 0 and the last expression divides by zero. That is undefined behaviour, and on x86 it takes SIGFPE.

Verified rather than inferred:

• On a host with no usable device, cudaGetDeviceProperties returns cudaErrorInsufficientDriver and writes no byte of the output struct. Probed by passing a buffer filled with 0xAA and comparing it afterwards; all 4096 bytes came back unchanged. cudaGetDevice also fails and leaves device at its initialiser, so the code proceeds with device 0.
• Running the same arithmetic over a zeroed struct exits 136.
• 34 families carry this computation. None of them check either return value.

Honest about the reach: getting here needs a CUDA build running where the query fails, and TensorRT would have failed to build or load an engine well before the sampler runs, so the practical exposure is small. It is still undefined behaviour on a path with no guard at all.

Recommended fix: check both CUDA calls and fail fast with the returned CUDA error before reading props or computing the policy. A device-query failure means this GPU sampler cannot run correctly, so it should surface as a clear runtime error rather than undefined arithmetic.

I do not recommend either of the earlier fallback shapes:

• Returning an empty policy avoids the host-side division, but the GPU helper then returns without writing d_token_id; the caller still copies and consumes that value, which can be stale or uninitialised.
• Clamping the grid to a positive lower bound preserves normal-device arithmetic, but silently invents a policy from invalid device metadata and lets later CUDA operations fail less clearly.

The mechanical change should therefore use the repository CUDA error-reporting convention (or an equivalent explicit status check) in every family carrying this computation. Normal successful-device values for total_threads and counter_offset must remain unchanged, with regression coverage for the failed-query path.

2. GPU tests report success when their fixture engine could not be built

auto engine = build_mock_decoder();
if (!engine) {
    std::cerr << "WARNING: Could not build mock decoder engine, skipping test
";
    return;
}

The case returns without touching the failure counter, so the binary exits 0 and ctest reports a pass.

• 224 of these across 32 test files. 223 of them, in 31 files, are cases whose test is registered in MODEL.toml with REQUIRES_TRT,REQUIRES_GPU. The one exception is tests/cpp/test_tvm_ffi_module_loader.cpp, which skips on an unset FLASHINFER_KERNEL_SO and is a legitimate opt-in.
• Because ctest already gates those 31 on TRT and a GPU, the skip only fires when the environment was declared capable and the fixture failed anyway. That is worth reporting, not swallowing.
• The 224 sites do not represent 224 decisions. The eleven encoder tests (albert, bert, convbert, deberta, distilbert, dpr, electra, modernbert, mpnet, roberta, xlnet) are byte-identical once the model name is normalised, which is 99 of the sites on its own. The VL and recurrent tests are separate files that reuse the same idiom.

The practical cost is that a green ctest -R <family> is not evidence that anything ran. I hit this directly on #1158: I reported ctest -R smollm3 as 4/4 passed on an H100 and could not show that the two GPU-labelled cases had not taken this path, because the pass looks the same either way.

Every one of the 31 files already defines check(bool, const char*) and a failures counter that main returns, so the change is uniform:

-        std::cerr << "SKIP encoder_embed
";
+        check(false, "test_encoder_embed: the fixture engine did not build");
         return;

This will turn some currently green runs red, which is the point, but it is a wide change and I would rather not land it unannounced.

What I would like to know

Please confirm whether these two recommended fixes match the intended failure policy. Once confirmed, I propose one mechanical cross-family PR per defect: one for explicit CUDA query failure handling, and one for making required GPU fixture-build failures fail their tests.