feat(generation): add TensorRT-LLM rollout backend#2420
Conversation
13a48f1 to
7cd4326
Compare
There was a problem hiding this comment.
thanks for adding this and overall looks clean! left some comments.
you may also need to fix the copyright-check and check lint by run pre-commit run --all-files locally.
Scope: skipped pyproject.toml, docker/Dockerfile, tools/build-custom-trtllm.sh, 3rdparty/TensorRT-LLM-workspace/*, and uv.lock per author instruction — those are still under discussion.
Focus areas covered: the new nemo_rl/models/generation/trtllm/*.py module (worker, generation interface, NCCL/IPC backend), the algorithm-layer integration in grpo.py and friends, and the PY_EXECUTABLES.TRTLLM registration.
85bb19a to
7f3a748
Compare
|
/ok to test 7f3a748 |
|
/ok to test 7f3a748 |
|
/ok to test 68f56cb |
|
/ok to test a90b543 |
|
/ok to test a5c8345 |
terrykong
left a comment
There was a problem hiding this comment.
Team review (6 agents + adversarial verification pass; all findings verified at head 68f56cb). The backend integration is solid — it closely mirrors the vLLM sibling, and we verified a number of scary-looking things are correct (NCCL rank math, IPC-ZMQ protocol/ACK pairing, logprob extraction, top_k=0 sentinel — vLLM's -1 would raise in TRT-LLM, the packed_tensor stream syncs, mcore×trtllm uv-conflict + per-actor venvs, and the full version-pin chain pyproject → workspace → _backend.py → build script → uv.lock → ccache tags, all agreeing on 1.3.0rc21 @ bf2ef86). The 14 inline comments are ranked; the ray.sub topology one is the only high-severity item.
Open items from earlier review threads (no new inline comments to avoid duplication):
- Rollout logger metrics:
TrtllmGenerationstill inherits the no-opget_logger_metrics/clear_logger_metrics, so throughput/KV-util metrics silently vanish for TRT-LLM runs. - EP nightly coverage: still unanswered; generation-side
moe_expert_parallel_sizeis unset in every recipe. - Recipe matrix + curves: see the partial-fix reply — both mcore recipes are colocated+sync rather than the agreed non-colocated+async-grpo, and convergence curves are still missing from the description.
PR description nits: the Recipes section still describes the old 4-config set ("Qwen3 1.7B / 8B…"); the actual set is Qwen2.5-0.5B + Qwen3-1.7B-fsdp2 + nanov3-30BA3B plus the grpo_math_1B_trtllm.yaml exemplar.
Non-blocking notes: a mid-refit exception can leave the IPC-ZMQ REP socket mid-state (trtllm_backend.py:190-273) — self-limiting since refit failure is already fatal, but worth a try/finally if refit ever becomes retryable · CMake 3.31→4.0.3 is image-global; this PR's own image builds are the evidence other CUDA extensions still compile · the Python-side init_ray SLURM/MPI strip was checked and is safe (topology comes from Ray custom resources registered before Python starts).
FYI — notable uv.lock changes (for readers skimming the 2.4k-line lock diff):
- The trtllm extra adds a fourth concurrent
transformersversion (5.5.4, alongside the existing 5.5.0/5.6.0/5.8.1 per-extra forks), plus per-extra forks oftorchao(0.15.0),nvidia-cutlass-dsl(4.5.0/4.5.1), andflashinfer-python(0.6.12) — expected given the declared extra conflicts, but each backend venv now runs different versions of these. - Two global bumps that affect all extras, not just trtllm:
zipp3.23.1 → 4.1.0 (major) andlibrt0.11.0 → 0.13.0. flash-attn-4enters at a beta pin (4.0.0b15) via tensorrt-llm.- ~45 new transitive packages ride in with
tensorrt-llm, including some heavyweight baggage worth knowing about:boto3/botocore(AWS SDK), an audio stack (librosa,audioread,audioop-lts),hydra-core,etcd-sdk-python,modelscope-hub,mpi4py, and thetensorrt10.16.1.11 suite (cu13 split packages pinned via hand-written[[tool.uv.dependency-metadata]]).
Generated by Claude Code
Signed-off-by: shuyixiong <219646547+shuyixiong@users.noreply.github.com>
Signed-off-by: Shiki Wu <shikiw@nvidia.com>
Signed-off-by: shuyixiong <219646547+shuyixiong@users.noreply.github.com>
Signed-off-by: Shiki Wu <shikiw@nvidia.com>
Signed-off-by: Shiki Wu <shikiw@nvidia.com>
Signed-off-by: Shiki Wu <shikiw@nvidia.com>
Signed-off-by: Jonas Yang <joyang@nvidia.com>
Signed-off-by: Jonas Yang <joyang@nvidia.com>
Signed-off-by: Jonas Yang <joyang@nvidia.com>
Signed-off-by: Jonas Yang <joyang@nvidia.com>
Signed-off-by: Jonas Yang <joyang@nvidia.com>
Signed-off-by: Jonas Yang <joyang@nvidia.com>
Signed-off-by: Jonas Yang <joyang@nvidia.com>
Signed-off-by: Jonas Yang <joyang@nvidia.com>
Signed-off-by: Jonas Yang <joyang@nvidia.com>
|
/ok to test 340c8b5 |
terrykong
left a comment
There was a problem hiding this comment.
Build sense-check — TRT-LLM caching (joyang-nv's changes)
Focused review of the TRT-LLM build/caching work only (docker/Dockerfile, tools/build-custom-trtllm.sh, .github/workflows/_build_container.yml, 3rdparty/TensorRT-LLM-workspace/_backend.py). Static design review — the ~60-min compile was not run locally (no GPU).
Verdict: the design is sound and I found no correctness bugs. In particular:
- The layer ordering genuinely achieves the TE-reuse goal: the TE-building syncs run in one layer before the TRT-LLM inputs are
COPY'd, and a[tool.trtllm].refbump changes neitheruv.locknor the rootpyproject, so it only busts the last layer. - The wheel cache key in
_backend.pycorrectly folds url+ref+version+real platform tag+arch+torch/CUDA toolchain. CCACHE_COMPILERCHECK=contentand the incremental-vs-compact registry layering with a nonce-forced export RUN are correct, and theassert_patch_targetfail-loud guards are the right instinct.
The inline comments are the few things worth eyeballing before trusting it in prod — none are blockers. The lead item (unconditional compile) is the one that actually matters for a build sense-check.
Generated by Claude Code
Signed-off-by: Jonas Yang <joyang@nvidia.com>
|
/ok to test e927101 |
Signed-off-by: Jonas Yang <joyang@nvidia.com>
|
/ok to test b479c39 |
Signed-off-by: Jonas Yang <joyang@nvidia.com>
|
/ok to test c9dd7ec |
What does this PR do?
Adds TensorRT-LLM (TRT-LLM) as a new generation backend for RL rollouts. This lets GRPO training drive high-throughput TRT-LLM inference with in-flight weight updates (refit), in both colocated and non-colocated deployment modes.
Key changes
Generation backend (
nemo_rl/models/generation/trtllm/)trtllm_generation.py—TrtllmGenerationimplementingGenerationInterface. Manages Ray worker groups with tensor parallelism, dispatches samples round-robin across data-parallel shards, exposesgenerate()/generate_async()/prepare_for_generation()/finish_generation(), and handles colocated sleep/wake for GPU memory sharing.trtllm_worker_async.py—TrtllmAsyncGenerationWorkerRay actor wrappingAsyncLLM; concurrent per-sample generation andcollective_rpchooks for weight updates.trtllm_backend.py—NcclExtensionWorkerExtensioninjected into TRT-LLM'sRayGPUWorker. NCCL collective broadcasts for non-colocated refit; CUDA IPC + ZMQ weight streaming for colocated refit; wraps reload with stream sync + prefix-cache invalidation.config.py—TrtllmConfig(extendsGenerationConfig) andTrtllmSpecificArgs(TP, MoE expert parallelism, async engine mode, in-flight weight updates, memory/engine knobs).Algorithm / rollout wiring
grpo.py:trtllmbackend case in worker init, async-rollout detection, and refit path (shared ZMQ IPC with vLLM). NeMo-gym raisesNotImplementedErrorfor trtllm (no HTTP server yet — deferred to a follow-up agent-RL PR).async_utils/trajectory_collector.pyandexperience/rollouts.py: trtllm backend handling (async engine required).distributed/virtual_cluster.py+ray_actor_environment_registry.py: newPY_EXECUTABLES.TRTLLMexecutable (uv run --locked --extra trtllm) and worker registration.utils/packed_tensor.py: addedstream.synchronize()after NCCL broadcasts.Build & dependencies
pyproject.toml: newtrtllmextra pinningtensorrt-llm==1.3.0rc21, path source into3rdparty/TensorRT-LLM-workspace, no-build-isolation config, and conflict declarations betweentrtllmandvllm/sglang/automodel/mcore.3rdparty/TensorRT-LLM-workspace/: custom PEP 517 build backend (_backend.py) with content-addressed wheel caching +tools/build-custom-trtllm.shthat clones and compiles TRT-LLM (x86_64 and aarch64).docker/Dockerfile: ccache, CMake 4.0.3, TRT-LLM build args, and aSKIP_TRTLLM_BUILDflag to conditionally skip TRTLLM build and dependency.Recipes
Adds a TRT-LLM generation exemplar plus three recipes that inherit from it:
wandb link: https://wandb.ai/nvidia/nemo-rl/runs/b5r05hnd?nw=nwusershikiwu
wandb link: https://wandb.ai/nvidia/nemo-rl/runs/upc7lm0j
wandb link: https://wandb.ai/nvidia/nemorl-trtllm-run/groups/trtllm-nanov3-dapo
Before your PR is "Ready for review"
Pre checks:
Additional Information