Skip to content

feat(qec): add cuda_device_id placement knob for GPU decoders#690

Merged
melody-ren merged 1 commit into
NVIDIA:mainfrom
melody-ren:melodyr/decoder-gpu-pinning
Jul 13, 2026
Merged

feat(qec): add cuda_device_id placement knob for GPU decoders#690
melody-ren merged 1 commit into
NVIDIA:mainfrom
melody-ren:melodyr/decoder-gpu-pinning

Conversation

@melody-ren

@melody-ren melody-ren commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Carrying the content of #664 , and its commit 7fe9529 in which the cuda_device_id knob was introduced. This PR is meant to revert 664's scope change introduced by 19b95bf.

This PR should be followed by #678 to enable support for the decoding server.

PR description copied verbatim from #664 :

Summary

First PR of the #634 split (per @bmhowe23's review): cuda_device_id only — GPU decoder pinning, settable at construction via kwargs and the YAML realtime config. NUMA/mempolicy/cpu_affinity and thread-binding APIs are deferred to a follow-up draft PR (next release), Python interfaces for those to a third.

What it does

  • decoder::get() reads and validates cuda_device_id (negative or >= device count → std::runtime_error), persistently pins the constructing thread (cudaSetDevice, no restore), strips the key before the plugin constructor, and stores it (get_cuda_device_id(), -1 = unpinned).
  • Model: one thread owns one decoder. The thread that creates a decoder drives its decode calls; construction-time and lazy decode-time allocations all land on the pinned device with zero plugin changes — covers trt_decoder and the closed-source nv-qldpc-decoder transparently. This addresses the lazily-allocating-decoder concern from the Add hardware pinning for QEC decoders #634 review without per-call guards or new decode entry points.
  • decode_async() is the sole exception: its fresh std::async worker pins itself for the call's duration via a lib-private RAII CudaDeviceGuard (libs/qec/lib/hardware_guards.h, header-only; PR2 extends it).
  • YAML realtime config: cuda_device_id is a top-level decoder_config field (next to type/transport — placement knob common to any GPU decoder, not a per-decoder algorithm arg). Surfaced by prepare_decoder_params() for every decoder type; also exposed as decoder_config.cuda_device_id in Python.
  • Realtime session: the host dispatcher (one thread, all decoders) applies each decoder's device set-if-different (no restore) before enqueue and before DEVICE-mode graph capture; no-op for unpinned decoders.
  • Python kwargs work with no binding changes: qec.get_decoder("trt_decoder", H, cuda_device_id=1).

To place N decoders on N GPUs, create each decoder on its own thread (std::async / threading.Thread) — documented in realtime_decoding.rst.

Testing

  • 7 new C++ unit tests (DecoderCudaDeviceId.*): kwarg contract (absent/negative/out-of-range), key stripped before strict-validating plugin ctors, persistent pin observable after construction, two decoders on two GPUs from two threads, async worker pinning itself (verified RED→GREEN on a 2-GPU machine). GPU tests skip below the needed device count.
  • YAML: round-trip with the new field; prepare_decoder_params surfaces the knob for trt and non-trt types (ordering vs the trt-only early return is locked by test).
  • Python: kwargs error path (cuda_device_id named in the error) and valid-id construct+decode. Full test_decoder.py 61/61.
  • Full suites: test_decoders 52/52, test_decoders_yaml 20 pass / 2 skips (nv-qldpc-gated).

Notes for reviewers

  • Known pre-existing quirk (not introduced here): negative int kwargs from Python surface as an opaque bad_cast because hetMapFromKwargs stores Python ints as size_t (libs/core kwargs_utils.h) — the friendly negative-id message is only reachable from C++/YAML. Follow-up candidate in core.
  • nv-qldpc-decoder internal changes (mirroring the trt pattern) are a separate follow-up.

Supersedes #663 (same content, reopened from the fork).

🤖 Generated with Claude Code

Decoders can now be pinned to a specific CUDA device at construction
via a cuda_device_id parameter, settable through C++/Python kwargs
and the YAML realtime config (top-level decoder field, next to type/
transport).

Model: one thread owns one decoder. decoder::get() validates the id
(negative or >= device count throws), persistently pins the
constructing thread with cudaSetDevice (no restore), strips the key
before the plugin constructor, and stores it (get_cuda_device_id()).
Plugin constructors therefore allocate on the right device with zero
plugin changes -- this covers trt_decoder and the closed-source
nv-qldpc-decoder transparently.

decode_async() is the one exception: its fresh std::async worker pins
itself for the call's duration via a lib-private RAII CudaDeviceGuard
(libs/qec/lib/hardware_guards.h).

The realtime host dispatcher (one thread serving all decoders) applies
each decoder's device with set-if-different before enqueue and before
DEVICE-mode graph capture; no-op for unpinned decoders.

Tests: 7 C++ unit tests incl. 2-GPU placement and async-worker pinning,
YAML round-trip + prepare_decoder_params coverage, Python kwargs tests.

NUMA/mempolicy/cpu_affinity and thread-binding APIs are deferred to a
follow-up PR per review feedback on NVIDIA#634.

Signed-off-by: kvmto <kmato@nvidia.com>
Signed-off-by: Melody Ren <melodyr@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 13, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@melody-ren

Copy link
Copy Markdown
Collaborator Author

/ok to test 7e2e846

@melody-ren
melody-ren marked this pull request as ready for review July 13, 2026 20:46
@melody-ren
melody-ren enabled auto-merge (squash) July 13, 2026 20:51
@melody-ren
melody-ren merged commit 2cd873b into NVIDIA:main Jul 13, 2026
31 of 40 checks passed
@melody-ren
melody-ren deleted the melodyr/decoder-gpu-pinning branch July 13, 2026 23:11
melody-ren added a commit that referenced this pull request Jul 13, 2026
Summary:

- Worker threads in the decoding server now pin themselves to the
decoder's cuda_device_id before serving. Construction only pinned the
registry thread, so every worker was decoding on the default device
regardless of the pin prior to this patch.
- The direct call path (no realtime session) had the same problem:
configure_decoders leaves the thread on the last decoder's device, so
decoder 0 would decode on decoder N's GPU. Each decode now selects its
own decoder's device first.
- If a pin can't be honored we fail instead of decoding on the wrong
device: a worker that can't pin aborts server startup, and a
cudaSetDevice failure during dispatch returns an error response instead
of logging a warning and carrying on.
- Hololink: HOLOLINK_GPU_ID and cuda_device_id both name the GPU the
FPGA is attached to, so they must agree. Either one alone selects the
device; setting both to different values throws at transport creation.
Single decoder only, same as the rest of the gpu_roce path.
- Nothing changes if cuda_device_id is not set.
- Decoder construction is transactional: if a plugin constructor throws
after its device was selected, the calling thread's CUDA device is
restored to its previous value instead of being left on the failed
decoder's device. Originally first introduced in #687. Adopting the
change here.

Outdated: This is a follow-up PR to #664, which should be merged before
this PR

This PR depends on #690

---------

Signed-off-by: kvmto <kmato@nvidia.com>
Signed-off-by: Melody Ren <melodyr@nvidia.com>
Co-authored-by: kvmto <kmato@nvidia.com>
bmhowe23 added a commit to bmhowe23/cudaqx that referenced this pull request Jul 14, 2026
Bring in merged main, including the cuda_device_id GPU-pinning knob (NVIDIA#690)
and its follow-up fixes (NVIDIA#678). NVIDIA#690's cuda_device_id additions
(decoder_config field, YAML mapOptional, Python def_rw, the two YAML
tests, and the realtime_decoding.rst example/prose) merge cleanly into
the schema-registry config on this branch and are absorbed here.

The docs conflict (this branch's decoder_custom_args schema section vs
main's cuda_device_id section) is resolved by keeping both.

Two branch-side adaptations NVIDIA#690 could not anticipate are layered on in
the following commit: exporting cuda_device_id in this branch's
decoder_config_json_schema(), and adapting NVIDIA#690's trt YAML test to the
removed trt_decoder_config struct.

Signed-off-by: Ben Howe <bhowe@nvidia.com>
bmhowe23 added a commit to bmhowe23/cudaqx that referenced this pull request Jul 14, 2026
Two branch-specific adaptations for the cuda_device_id knob (merged from
NVIDIA#690) that NVIDIA#690 could not make, because they touch code that only exists
on this branch:

- config.cpp: export cuda_device_id in decoder_config_json_schema(). This
  JSON Schema export is a branch feature NVIDIA#690 predates; its envelope must
  stay in sync with MappingTraits<decoder_config> (enforced by the
  in-file contract), so the newly mapped key is added here.
- test_decoders_yaml.cpp: adapt NVIDIA#690's PrepareDecoderParamsSurfacesCudaDeviceId
  trt case, which assigned a trt_decoder_config{} struct this branch
  removed. prepare_decoder_params only manipulates the params map (no
  schema or filesystem access), so empty custom args exercise the trt
  path equivalently.

Signed-off-by: Ben Howe <bhowe@nvidia.com>
bmhowe23 added a commit to bmhowe23/cudaqx that referenced this pull request Jul 15, 2026
…ridge branch

Main landed the squashed PR 670 plus NVIDIA#656/NVIDIA#673/NVIDIA#674/NVIDIA#678/NVIDIA#680/NVIDIA#683/NVIDIA#688/
NVIDIA#690/NVIDIA#691/NVIDIA#692.  Resolution keeps this branch's design and ports main's
content into it:

- Naming: main's GpuRoce{Transceiver,Factory,LinkCheck} map 1:1 onto this
  branch's DeviceGraph* files (the de-transport-ing follow-up requested in
  the PR 670 review).  Main's post-review fixes are ported into the
  renamed files: ring-size overflow + host-page-size alignment validation,
  gpu_id resolved from the decoder's cuda_device_id instead of an env var,
  the factory taking pinned_cuda_device, and the linkcheck exercising the
  new factory signature.
- Schema: per-decoder 'dispatch: host|device_graph' plus the top-level
  transport section stay; main's per-decoder 'transport:' key does not
  return.  cuda_device_id is adopted as a per-decoder placement knob
  (config struct + YAML trait), and the hsb script now injects
  'dispatch: device_graph' + cuda_device_id and drives the server with
  QEC_DEVICE_GRAPH_* env and the device_graph READY sentinel.
- Features adopted from main: decoder CUDA-device pinning (worker-thread
  pin via promise, graph-capture pin, hardware_guards.h), per-session
  decode counters + print_session_stats + QEC_DECODING_SERVER_STATS
  server-side stats printing, DecodingServer ctor exception safety,
  virtualized realtime decoder API (NVIDIA#674), CMP0126 fix, CUDA::cudart on
  the core lib, and the cudevice-archive dedup in the server link.
- DecodingSession combines main's set_graph_capture_device with this
  branch's reserved-SMs decode-graph capture
  (QEC_DEVICE_GRAPH_RESERVED_SMS).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Ben Howe <bhowe@nvidia.com>
anjbur added a commit to anjbur/cudaqx that referenced this pull request Jul 16, 2026
…NVIDIA#690)"

This reverts commit 2cd873b.

Signed-off-by: Angela Burton <angelab@nvidia.com>
anjbur added a commit to anjbur/cudaqx that referenced this pull request Jul 16, 2026
…ransport integration

Signed-off-by: Angela Burton <angelab@nvidia.com>
anjbur added a commit to anjbur/cudaqx that referenced this pull request Jul 16, 2026
…VIDIA#609)". Resolve NVIDIA#609 revert conflicts by restoring the legacy host-call and direct-decoding paths while retaining the GPU device-placement fixes from NVIDIA#690 and NVIDIA#678.

This reverts commit 927ec7f.

Signed-off-by: Angela Burton <angelab@nvidia.com>
melody-ren added a commit that referenced this pull request Jul 17, 2026
…698)

## Background
- #690 introduced the cuda_device_id placement knob for GPU decoders.
- #678 made worker threads and the inproc graph capture honor the pinned
device, and pinned decoder construction.
- #683 closed the remaining gap: the gpu_roce decoding-server capture
path now pins too.

Each fix added a device pin to one more path via its own ad-hoc helper.
The "which device" decision ended up duplicated across dispatch,
capture, the worker, construction, and the gpu_roce transport. And that
is the motivation for this PR.

## What this PR does
Basically, every path should be following a single source of truth. 

Consolidate all of it to: the decoder's `cuda_device_id`, resolved one
way (`decode_device_for`) and applied through two sanctioned wrappers —
`pin_decode_device` (dispatch/decode) and `capture_graph_pinned` (graph
capture) — in `hardware_guards.h.` Every path now derives its device
from that one field; none selects a device by any other means. Unpinned
(< 0) keeps existing semantics: dispatch no-ops, capture defaults to
device 0.

Signed-off-by: Melody Ren <melodyr@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants