Skip to content

[NVIDIA] Add DSR1 TensorRT Support and Enhanced Plotting - #7

Merged
kimbochen merged 7 commits into
mainfrom
kepotdar-fix-chart-add-dsr1-trt
Sep 8, 2025
Merged

[NVIDIA] Add DSR1 TensorRT Support and Enhanced Plotting#7
kimbochen merged 7 commits into
mainfrom
kepotdar-fix-chart-add-dsr1-trt

Conversation

@kedarpotdar-nv

Copy link
Copy Markdown
Collaborator

Summary

This PR adds TensorRT-LLM FP4 support for DSR1 model and enhances the plotting system to better handle multiple model variants and precision types.

Changes Made

1. DSR1 Template Enhancements

✅ Added precision parameter to DSR1 template with fp8 default
✅ Added b200-trt job for DSR1 with TensorRT support
✅ Set b200-trt to use fp4 precision specifically
✅ Updated collect-results to include b200-trt job

2. TRT-LLM Configuration

✅ Created dsr1_b200_trt_slurm.sh benchmark script
✅ Uses TensorRT-LLM with trtllm-serve command
✅ Configured for DSR1 FP4 model (nvidia/DeepSeek-R1-0528-FP4)
✅ MTP support

3. Plotting System Improvements

✅ Enhanced model grouping - groups by model family (70b, dsr1) instead of full model names
✅ Added precision distinction - different markers for fp8 (circles) vs fp4 (squares)
✅ Improved legend labels - shows precision in labels (e.g., "B200-TRT (fp4)")

New Features

  • DSR1 TensorRT benchmarking with fp4 precision
  • Visual precision distinction in plots (circles vs squares)
  • Improved model grouping for better chart organization

@kimbochen

Copy link
Copy Markdown
Collaborator

Thank you for the PR. Can you revert the tp-list to full sweep?

@kedarpotdar-nv

Copy link
Copy Markdown
Collaborator Author

Done.

@kimbochen
kimbochen merged commit e4e60be into main Sep 8, 2025
@kedarpotdar-nv
kedarpotdar-nv deleted the kepotdar-fix-chart-add-dsr1-trt branch September 18, 2025 00:57
@cquil11 cquil11 added the NVIDIA label Apr 8, 2026
@cquil11 cquil11 changed the title Add DSR1 TensorRT Support and Enhanced Plotting [NVIDIA] Add DSR1 TensorRT Support and Enhanced Plotting Apr 8, 2026
Oseltamivir added a commit that referenced this pull request Jun 23, 2026
Add summarize.py (compact NCCL/DeepEP results table, printed at end of every job) and make it the result gate. Fix review findings: benchmark failures/skipped-deepep now fail the job instead of reporting green (#1); DeepEP nodes from SLURM_NNODES not world_size//8 (#3); apply Buffer.set_num_sms so num_comm_sms is real (#8); nccl-tests -c 1 with a missing check footer is now invalid (#7); use context managers for file reads (#4,#5); launchers export COLLECTIVEX_IMAGE/_DIGEST for provenance (#9); trim workflow_dispatch sku options to launcher-backed pools (#2). Artifact-path finding (#6) already fixed via cx_collect_results.
Oseltamivir added a commit that referenced this pull request Aug 7, 2026
queries.md #7. A class-level attribute naming another attribute, so seven shared getattr/setattr
sites could reach a field one adapter spelled differently. It bought nothing: the only divergence
was nccl-ep calling it combine_input_t, and nothing outside our own code ever saw the name.

Verified before renaming, because the "_t" suffix looks like it might be load-bearing:
- The nccl handle is adapter-owned. It is our namespace object holding adapter fields plus
  h.handle, which is the actual library group, so nccl.ep never observes this field's name.
- The value genuinely is not a torch tensor -- it is an nccl.ep wrapper, which is what the
  suffix connoted -- but every shared site only passes it through opaquely. The rename keeps
  that property, and the comment now states it where the field is set rather than encoding it
  in the spelling.
- No collision: h.combine_in_t is a DIFFERENT field (the dispatch output that HT slices), and it
  stays. The two names differing by one character is itself an argument for the rename.
- The locally-untracked tests/test_ep_nccl_handle.py does not reference the field.

So: nccl-ep's stage() sets handle.combine_input like every other adapter, the base declaration and
flashinfer's redundant re-declaration of the default both go, and the six getattr/setattr sites
become direct attribute access.

The test that asserted the indirection was tautological -- it set combine_input_attr to
"combine_input" and then checked the machinery honoured it, which is true by construction. It is
now the behavioural claim that matters: a staged tensor reaches combine. 163 tests green.
xiaohuguo2023 added a commit that referenced this pull request Aug 19, 2026
Move the K3 FP4 MI355X recipe onto vllm/vllm-openai-rocm:nightly-8efa13b700f1836657699cae2503dc2feab27fa0
(vLLM 0.27.2rc1.dev122+g8efa13b70, ROCm 7.2.3): 3 image: pins in
configs/amd-master.yaml (:627/:1296/:1325) + doc/comment tag copies +
docs/waiver/2508.md. Also carries the native) KV-offload arm in the base
entry script.

WIP — NOT yet validated on the new image: 0.27 rewrote rocm_aiter_mla.py,
so the patch orchestrator dies at 5/9 (patch_fp8asm.py padded_q anchor).
Patch reconciliation (drop upstreamed patches #1/#5/#6 + trio #2/#3/#4,
keep #7 offload eagle veto, re-port dspark 4/7,4c,5/7) + conc-1 DSpark
boot/accuracy + perf compare pending. Do not merge until green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants