refactor: Remove IGptDecoderBatched interface #5577
Conversation
|
/bot run |
|
PR_Github #10241 [ run ] triggered by Bot |
|
PR_Github #10241 [ run ] completed with state |
Funatiq
force-pushed
the
dev/remove_i_gpt_decoder_batched
branch
from
442d37e to
25815e3
Compare
|
/bot run |
|
PR_Github #10293 [ run ] triggered by Bot |
|
PR_Github #10293 [ run ] completed with state |
Funatiq
force-pushed
the
dev/remove_i_gpt_decoder_batched
branch
from
25815e3 to
66deecc
Compare
|
/bot run |
|
PR_Github #10365 [ run ] triggered by Bot |
|
PR_Github #10365 [ run ] completed with state |
Funatiq
force-pushed
the
dev/remove_i_gpt_decoder_batched
branch
from
66deecc to
7737645
Compare
|
/bot run |
|
PR_Github #10384 [ run ] triggered by Bot |
|
PR_Github #10384 [ run ] completed with state |
|
/bot run --disable-fail-fast |
|
PR_Github #10390 [ run ] triggered by Bot |
|
PR_Github #10390 [ run ] completed with state |
|
/bot run |
|
PR_Github #10460 [ run ] triggered by Bot |
|
PR_Github #10460 [ run ] completed with state |
There was a problem hiding this comment.
Pull Request Overview
Refactors the GPT decoder batching system by removing the deprecated IGptDecoderBatched interface, consolidating input definitions, and updating all includes and signatures to use the new structure.
- Removed
iGptDecoderBatched.hand all references to the old interface. - Inlined
decoder_batch::InputintogptDecoderBatched.hand updated constructors. - Updated includes in tests, runtime, and PyBind code to reference
decoderState.h,gptDecoder.h, andgptDecoderBatched.hinstead of the old interface header.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| cpp/tests/runtime/gptDecoderBatchedTest.cpp | Removed include of runtimeKernels.h, added decoderState.h and gptDecoder.h. |
| cpp/tensorrt_llm/runtime/gptDecoderBatched.cpp | Added include for gptDecoder.h to match new implementation. |
| cpp/tensorrt_llm/pybind/runtime/bindings.cpp | Dropped iGptDecoderBatched.h, added decoderState.h. |
| cpp/tensorrt_llm/pybind/batch_manager/algorithms.cpp | Added include of gptDecoderBatched.h. |
| cpp/tensorrt_llm/batch_manager/trtGptModelInflightBatching.cpp | Added decoderState.h and gptDecoder.h includes. |
| cpp/tensorrt_llm/batch_manager/makeDecodingBatchInputOutput.cpp | Switched from iGptDecoderBatched.h to gptDecoderBatched.h. |
| cpp/include/tensorrt_llm/runtime/iGptDecoderBatched.h | Removed entirely. |
| cpp/include/tensorrt_llm/runtime/gptDecoderBatched.h | Inlined decoder_batch::Input, removed interface inheritance and override specifiers. |
| cpp/include/tensorrt_llm/batch_manager/makeDecodingBatchInputOutput.h | Updated forward declarations and changed TensorPtr alias. |
| namespace decoder_batch | ||
| { | ||
|
|
||
| class Input |
There was a problem hiding this comment.
[nitpick] Consider extracting decoder_batch::Input into its own header (e.g., gptDecoderBatchedInput.h) to keep gptDecoderBatched.h focused on the decoder class and improve compilation modularity.
Copilot uses AI. Check for mistakes.
- Removed the IGptDecoderBatched interface and its references. - Moved decoder_batch::Input class to gptDecoderBatched.h. - Updated includes in relevant files to reflect the new structure. - Adjusted method signatures in GptDecoderBatched to remove interface dependencies. Signed-off-by: Robin Kobus <[email protected]>
- Removed unused includes from gptDecoderBatched.h. - Added decoderState.h include to trtGptModelInflightBatching.cpp and bindings.cpp to support new decoder state functionality. Signed-off-by: Robin Kobus <[email protected]>
- Removed gptDecoder.h include from gptDecoderBatched.h. Used forward declaration instead. - Added gptDecoder.h include to trtGptModelInflightBatching.cpp, and gptDecoderBatched.cpp to support new include structure. Signed-off-by: Robin Kobus <[email protected]>
Funatiq
force-pushed
the
dev/remove_i_gpt_decoder_batched
branch
from
7737645 to
309e124
Compare
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughRemoves IGptDecoderBatched interface, introduces runtime::decoder_batch::Input as a public type in gptDecoderBatched.h, adjusts MakeDecodingBatchInputOutput to new namespaces and ITensor-based alias, updates includes across batch_manager, bindings, and tests, and makes GptDecoderBatched a concrete class with updated method signatures. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant BM as BatchManager
participant M as MakeDecodingBatchInputOutput
participant GD as runtime::GptDecoderBatched
participant DS as decoder::DecoderState
participant IN as decoder_batch::Input
rect rgb(243,248,255)
note over M,GD: New public input surface
BM->>M: createDecoderBatchInputs(logits, batchSlots, ...)
M-->>BM: decoder_batch::Input (IN)
end
BM->>GD: forwardAsync(DS, IN)
activate GD
GD-->>BM: CudaEvent
deactivate GD
alt finalize step
BM->>GD: finalize(DS, batchSlot, samplingConfig, streaming)
GD-->>BM: CudaEvent
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
cpp/include/tensorrt_llm/runtime/gptDecoderBatched.h (2)
2-2: Update copyright year to 2025.According to the coding guidelines, all source files should have the NVIDIA copyright header with the current year.
- * Copyright (c) 2022-2024, NVIDIA CORPORATION. All rights reserved. + * Copyright (c) 2022-2025, NVIDIA CORPORATION. All rights reserved.
17-17: Missing include guard.According to the coding guidelines, header files must use include guards named
TRTLLM_<FILENAME>_Hwithout underscores prefix/suffix.The include guard should be:
-#pragma once +#ifndef TRTLLM_GPTDECODERBATCHED_H +#define TRTLLM_GPTDECODERBATCHED_HAnd at the end of the file (after line 159):
} // namespace tensorrt_llm::runtime + +#endif // TRTLLM_GPTDECODERBATCHED_Hcpp/tensorrt_llm/runtime/gptDecoderBatched.cpp (1)
2-2: Update copyright year to 2025.According to the coding guidelines, all source files should have the NVIDIA copyright header with the current year.
- * Copyright (c) 2022-2024, NVIDIA CORPORATION. All rights reserved. + * Copyright (c) 2022-2025, NVIDIA CORPORATION. All rights reserved.cpp/include/tensorrt_llm/batch_manager/makeDecodingBatchInputOutput.h (1)
56-63: Align header and implementation signatures for consistencyThe
createDecoderBatchInputsparameters differ between the declaration and definition. Please update them so names match exactly:• In
makeDecodingBatchInputOutput.hthe third parameter is namedlogits, but in the.cppit’sdecoderLogits.
– Change the implementation signature to:
cpp std::unique_ptr<tr::decoder_batch::Input> MakeDecodingBatchInputOutput::createDecoderBatchInputs( std::vector<SizeType32> const& activeSlots, runtime::decoder::DecoderState const& decoderState, std::vector<TensorPtr> const& logits, // ← use “logits” here SizeType32 maxNumSequences, std::vector<TensorPtr> const& batchSlots)
• (Optional) Verify the return‐type qualifier—declaration usesruntime::decoder_batch::Inputwhile definition usestr::decoder_batch::Input. Iftris an alias forruntime, consider using the same qualifier in both places for clarity.
♻️ Duplicate comments (2)
cpp/include/tensorrt_llm/runtime/gptDecoderBatched.h (1)
49-98: Consider extractingdecoder_batch::Inputinto its own header file.As noted by a previous reviewer, the
Inputclass is substantial enough to warrant its own header file (e.g.,gptDecoderBatchedInput.h). This would improve compilation modularity and keepgptDecoderBatched.hfocused on the decoder class itself.cpp/include/tensorrt_llm/batch_manager/makeDecodingBatchInputOutput.h (1)
50-50: TensorPtr alias choice is acceptable; mirror Input::TensorPtr in implementation for semantic clarity.Keeping TensorPtr as ITensor::SharedPtr here avoids needing the full Input definition in this header. In the .cpp (where gptDecoderBatchedInput.h is included), consider aliasing to decoder_batch::Input::TensorPtr to reflect the batched-input API semantics at the call sites.
Example in the .cpp (no diff here since it’s outside this header):
using TensorPtr = runtime::decoder_batch::Input::TensorPtr;
🧹 Nitpick comments (5)
cpp/tensorrt_llm/nanobind/batch_manager/bindings.cpp (1)
507-509: Consider adding parameter validation for consistency.The function creates a
decoder_batch::Inputbut doesn't validate thatlogitsVecis non-empty or that its structure matches expectations. While thedecoder_batch::Inputconstructor performs some validation, consider adding defensive checks here for consistency with other functions in this file.auto decodingInput = std::make_unique<tr::decoder_batch::Input>(logitsVec, 1); +TLLM_CHECK_WITH_INFO(!logitsVec.empty() && !logitsVec[0].empty(), + "Expected non-empty logits vector for decoding input"); decodingInput->batchSlots = batchSlots;cpp/include/tensorrt_llm/runtime/gptDecoderBatched.h (1)
59-61: Consider using braces for single-statement if blocks.According to the coding guidelines, always use brace-delimited bodies for control structures.
- TLLM_CHECK_WITH_INFO( - logits.size() == static_cast<size_t>(maxDecoderSteps), "logits vector size does not match maxDecoderSteps"); + { + TLLM_CHECK_WITH_INFO( + logits.size() == static_cast<size_t>(maxDecoderSteps), "logits vector size does not match maxDecoderSteps"); + }cpp/tests/runtime/gptDecoderBatchedTest.cpp (2)
670-673: Fix minor typo: beamWdith -> beamWidth.Avoids micro-confusion and keeps naming consistent with the rest of the file.
- for (auto const beamWdith : beamConfig.beamWidths) + for (auto const beamWidth : beamConfig.beamWidths) { - name.append("Bw" + std::to_string(beamWdith)); + name.append("Bw" + std::to_string(beamWidth)); }- for (auto const beamWdith : beamConfig.beamWidths) + for (auto const beamWidth : beamConfig.beamWidths) { - name.append("Bw" + std::to_string(beamWdith)); + name.append("Bw" + std::to_string(beamWidth)); }Also applies to: 781-783
716-717: Remove unused variable.normalizeLogProbs is defined but never used in this test. Safe to drop.
- bool const normalizeLogProbs{true};cpp/include/tensorrt_llm/batch_manager/makeDecodingBatchInputOutput.h (1)
26-37: Forward declarations are fine; consider explicit dependency include and header guard.
- The forward decls for runtime::decoder::DecoderState and runtime::decoder_batch::Input are correct and help decouple headers.
- However, this header uses ITensor::SharedPtr (Line 50) without explicitly including iTensor.h; relying on transitive includes from runtime/common.h is brittle. Please include tensorrt_llm/runtime/iTensor.h directly.
- Per coding guidelines, header files must use include guards named TRTLLM__H. This header currently uses only pragma once.
Apply this diff to add the guard and the missing include:
@@ -#pragma once +#pragma once +#ifndef TRTLLM_MAKEDECODINGBATCHINPUTOUTPUT_H +#define TRTLLM_MAKEDECODINGBATCHINPUTOUTPUT_H @@ -#include "tensorrt_llm/runtime/modelConfig.h" +#include "tensorrt_llm/runtime/modelConfig.h" +#include "tensorrt_llm/runtime/iTensor.h" @@ } // namespace tensorrt_llm::batch_manager + +#endif // TRTLLM_MAKEDECODINGBATCHINPUTOUTPUT_H
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
cpp/include/tensorrt_llm/batch_manager/makeDecodingBatchInputOutput.h(2 hunks)cpp/include/tensorrt_llm/runtime/gptDecoderBatched.h(4 hunks)cpp/include/tensorrt_llm/runtime/iGptDecoderBatched.h(0 hunks)cpp/tensorrt_llm/batch_manager/makeDecodingBatchInputOutput.cpp(1 hunks)cpp/tensorrt_llm/batch_manager/trtGptModelInflightBatching.cpp(1 hunks)cpp/tensorrt_llm/nanobind/batch_manager/bindings.cpp(1 hunks)cpp/tensorrt_llm/nanobind/runtime/bindings.cpp(0 hunks)cpp/tensorrt_llm/pybind/batch_manager/algorithms.cpp(1 hunks)cpp/tensorrt_llm/pybind/runtime/bindings.cpp(0 hunks)cpp/tensorrt_llm/runtime/gptDecoderBatched.cpp(1 hunks)cpp/tests/runtime/gptDecoderBatchedTest.cpp(1 hunks)
💤 Files with no reviewable changes (3)
- cpp/tensorrt_llm/nanobind/runtime/bindings.cpp
- cpp/tensorrt_llm/pybind/runtime/bindings.cpp
- cpp/include/tensorrt_llm/runtime/iGptDecoderBatched.h
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{cpp,cxx,cc,cu,h,hpp,hxx,hh,cuh}
📄 CodeRabbit inference engine (CODING_GUIDELINES.md)
**/*.{cpp,cxx,cc,cu,h,hpp,hxx,hh,cuh}: In C++, close namespaces with a comment naming the namespace (e.g., } // namespace foo)
Prefer const/constexpr variables over #define for constants
Declare variables const if not modified after initialization
Use Allman brace style in C++
C++ filenames use lowerCamelCase and must be case-insensitively unique within a build target
C++ type names use UpperCamelCase
Local variables, methods, and namespaces use lowerCamelCase
Global non-static variables not in anonymous namespace use gPrefix lowerCamelCase (e.g., gExample)
Static globals or globals in anonymous namespaces use sPrefix lowerCamelCase
Locally visible static variables start with 's' (e.g., static std::once_flag sFlag;)
Member variables use mPrefix lowerCamelCase; public members may omit but are encouraged to use 'm'
Constants (enums, global/static/function-scope magic numbers) use kPREFIXED_UPPER_SNAKE (e.g., kDIGIT_NUM)
If macros are unavoidable, use UPPER_SNAKE_CASE (prefer constants over #define)
Constructor parameter that conflicts with a public member name gets trailing underscore (foo_)
Literal suffixes should be uppercase (e.g., 1234L not 1234l)
C++: use spaces only; indent 4 spaces
Run clang-format (LLVM style) before submitting; wrap lines at 120 characters
If formatting must be bypassed, use // clang-format off/on around the section
Prefer smart pointers; use unique_ptr for sole ownership, shared_ptr for shared; weak_ptr only in exceptional cases
Do not use deprecated pre-C++11 smart pointers
Use C++ style comments; avoid C comments except special inline cases; prefer // single-line
Capitalize and punctuate full-sentence comments
Follow Doxygen rules: use //! for comments and //!< for members in C++
Disable code with #if/#endif and mnemonic conditions; avoid commented-out code; avoid dead code
Do not throw exceptions across library boundaries
Use least-forceful casts; avoid removing const/volatile; avoid C-style and functional casts (except constructors); p...
Files:
cpp/tests/runtime/gptDecoderBatchedTest.cppcpp/tensorrt_llm/pybind/batch_manager/algorithms.cppcpp/tensorrt_llm/runtime/gptDecoderBatched.cppcpp/tensorrt_llm/nanobind/batch_manager/bindings.cppcpp/tensorrt_llm/batch_manager/makeDecodingBatchInputOutput.cppcpp/tensorrt_llm/batch_manager/trtGptModelInflightBatching.cppcpp/include/tensorrt_llm/batch_manager/makeDecodingBatchInputOutput.hcpp/include/tensorrt_llm/runtime/gptDecoderBatched.h
**/*.{cpp,cxx,cc,cu}
📄 CodeRabbit inference engine (CODING_GUIDELINES.md)
**/*.{cpp,cxx,cc,cu}: Avoid literal values except for 0, nullptr, true, false; use named constexpr for other literals
Place semicolon of empty for/while loop on a new line
Always use brace-delimited bodies for switch/while/do-for/if/else
Use inline C comments in argument lists when parameter meaning is unclear (e.g., /* checkForErrors = */ false)
Do not use assignment in subexpressions (e.g., if (x = y) ... is forbidden)
Switch on enums should enumerate all values and omit default to catch new values at compile time
Structure switch statements; prohibit fallthrough except between empty cases; each case ends with break or throw; return at end of case not allowed; put break inside braces for compound case
Prefer anonymous namespaces over static for internal linkage of functions
Every defined function must be called at least once (no unused methods)
Files:
cpp/tests/runtime/gptDecoderBatchedTest.cppcpp/tensorrt_llm/pybind/batch_manager/algorithms.cppcpp/tensorrt_llm/runtime/gptDecoderBatched.cppcpp/tensorrt_llm/nanobind/batch_manager/bindings.cppcpp/tensorrt_llm/batch_manager/makeDecodingBatchInputOutput.cppcpp/tensorrt_llm/batch_manager/trtGptModelInflightBatching.cpp
**/*.{h,hpp,hxx,hh,cuh,cpp,cxx,cc,cu}
📄 CodeRabbit inference engine (CODING_GUIDELINES.md)
Parameter names must be consistent between declarations and definitions
Files:
cpp/tests/runtime/gptDecoderBatchedTest.cppcpp/tensorrt_llm/pybind/batch_manager/algorithms.cppcpp/tensorrt_llm/runtime/gptDecoderBatched.cppcpp/tensorrt_llm/nanobind/batch_manager/bindings.cppcpp/tensorrt_llm/batch_manager/makeDecodingBatchInputOutput.cppcpp/tensorrt_llm/batch_manager/trtGptModelInflightBatching.cppcpp/include/tensorrt_llm/batch_manager/makeDecodingBatchInputOutput.hcpp/include/tensorrt_llm/runtime/gptDecoderBatched.h
**/*.{cpp,cxx,cc,cu,h,hpp,hxx,hh,cuh,py}
📄 CodeRabbit inference engine (CODING_GUIDELINES.md)
Prepend NVIDIA copyright header (current year) to all source files
Files:
cpp/tests/runtime/gptDecoderBatchedTest.cppcpp/tensorrt_llm/pybind/batch_manager/algorithms.cppcpp/tensorrt_llm/runtime/gptDecoderBatched.cppcpp/tensorrt_llm/nanobind/batch_manager/bindings.cppcpp/tensorrt_llm/batch_manager/makeDecodingBatchInputOutput.cppcpp/tensorrt_llm/batch_manager/trtGptModelInflightBatching.cppcpp/include/tensorrt_llm/batch_manager/makeDecodingBatchInputOutput.hcpp/include/tensorrt_llm/runtime/gptDecoderBatched.h
**/*.{h,hpp,hxx,hh,cuh}
📄 CodeRabbit inference engine (CODING_GUIDELINES.md)
Header files must use include guards named TRTLLM__H without underscores prefix/suffix (e.g., TRTLLM_FOO_BAR_HELLO_H)
Files:
cpp/include/tensorrt_llm/batch_manager/makeDecodingBatchInputOutput.hcpp/include/tensorrt_llm/runtime/gptDecoderBatched.h
🧠 Learnings (5)
📚 Learning: 2025-08-12T10:28:57.320Z
Learnt from: CR
PR: NVIDIA/TensorRT-LLM#0
File: CODING_GUIDELINES.md:0-0
Timestamp: 2025-08-12T10:28:57.320Z
Learning: Applies to **/*.{cpp,cxx,cc,cu,h,hpp,hxx,hh,cuh} : Do not include C headers directly (use <cstdint> not <stdint.h>)
Applied to files:
cpp/include/tensorrt_llm/runtime/gptDecoderBatched.h
📚 Learning: 2025-08-12T10:28:57.320Z
Learnt from: CR
PR: NVIDIA/TensorRT-LLM#0
File: CODING_GUIDELINES.md:0-0
Timestamp: 2025-08-12T10:28:57.320Z
Learning: Applies to **/*.{cpp,cxx,cc,cu,h,hpp,hxx,hh,cuh} : Avoid #ifdef/#ifndef except include guards; prefer #if defined(...) / #if !defined(...)
Applied to files:
cpp/include/tensorrt_llm/runtime/gptDecoderBatched.h
📚 Learning: 2025-08-12T10:28:57.320Z
Learnt from: CR
PR: NVIDIA/TensorRT-LLM#0
File: CODING_GUIDELINES.md:0-0
Timestamp: 2025-08-12T10:28:57.320Z
Learning: Applies to **/*.{cpp,cxx,cc,cu,h,hpp,hxx,hh,cuh} : Prefer smart pointers; use unique_ptr for sole ownership, shared_ptr for shared; weak_ptr only in exceptional cases
Applied to files:
cpp/include/tensorrt_llm/runtime/gptDecoderBatched.h
📚 Learning: 2025-08-12T10:28:57.320Z
Learnt from: CR
PR: NVIDIA/TensorRT-LLM#0
File: CODING_GUIDELINES.md:0-0
Timestamp: 2025-08-12T10:28:57.320Z
Learning: Applies to **/*.{cpp,cxx,cc,cu,h,hpp,hxx,hh,cuh} : Avoid C library functions when possible; use brace initialization or std::fill_n instead of memset for non-PODs
Applied to files:
cpp/include/tensorrt_llm/runtime/gptDecoderBatched.h
📚 Learning: 2025-08-15T06:46:54.897Z
Learnt from: eopXD
PR: NVIDIA/TensorRT-LLM#6767
File: cpp/tensorrt_llm/batch_manager/kvCacheManager.cpp:0-0
Timestamp: 2025-08-15T06:46:54.897Z
Learning: In cpp/tensorrt_llm/batch_manager/kvCacheManager.cpp addToken function, newly allocated blocks are unshared by design. The beam search path in addToken (when sequence.getNumTokens() > windowSize) is currently broken/non-functional with SWA, so the block allocation doesn't follow a shared-then-unshared pattern.
Applied to files:
cpp/include/tensorrt_llm/runtime/gptDecoderBatched.h
🔇 Additional comments (7)
cpp/tensorrt_llm/nanobind/batch_manager/bindings.cpp (1)
28-28: LGTM! Include forruntime::decoder::DecoderStateadded correctly.The addition of this include is necessary for the code on line 449 which directly references
runtime::decoder::DecoderState.cpp/include/tensorrt_llm/runtime/gptDecoderBatched.h (1)
103-110: LGTM! Clean API simplification after removing IGptDecoderBatched interface.The removal of the inheritance and the addition of proper type aliases creates a cleaner, more focused API. The use of [[nodiscard]] attributes for getters is a good practice.
cpp/tensorrt_llm/pybind/batch_manager/algorithms.cpp (1)
31-31: LGTM! Include added in correct order.The include for
gptDecoderBatched.his correctly placed afterdecoderState.h, maintaining alphabetical ordering within the runtime headers group.cpp/tensorrt_llm/batch_manager/makeDecodingBatchInputOutput.cpp (1)
25-25: LGTM! Include updated to use the new header.The change from
iGptDecoderBatched.htogptDecoderBatched.hcorrectly reflects the removal of the interface and the new API structure.cpp/tensorrt_llm/runtime/gptDecoderBatched.cpp (1)
29-29: LGTM! Include for gptDecoder.h added correctly.The addition of this include is necessary since
GptDecoderBatchedno longer inherits fromIGptDecoderBatchedand needs direct access toIGptDecoderdeclarations for the implementation.cpp/tests/runtime/gptDecoderBatchedTest.cpp (1)
27-28: Includes are explicitly required and correct.The test file invokes
decoder.getUnderlyingDecoder().setup(…)and heavily usesdecoder::DecoderStatethroughout (e.g., lines 105, 126, 198, 320, 454, 603) as shown by your ripgrep output.
–decoderState.hdefinesdecoder::DecoderState(nvidia.github.io)
–gptDecoder.hprovides the decoder interface withgetUnderlyingDecoder()(nvidia.github.io)These explicit includes remove any reliance on transitive headers and correctly expose the required symbols for compilation and clarity.
cpp/tensorrt_llm/batch_manager/trtGptModelInflightBatching.cpp (1)
58-60: Good include adjustments to match new public API.Explicitly including runtime/decoderState.h and runtime/gptDecoder.h here is the right call, since this TU constructs DecoderState and references GptDecoder through GptDecoderBatched::getUnderlyingDecoder(). This de-risks transitive-include breakage.
Description
Test Coverage
GitHub Bot Help
/bot [-h] ['run', 'kill', 'skip', 'reuse-pipeline'] ...Provide a user friendly way for developers to interact with a Jenkins server.
Run
/bot [-h|--help]to print this help message.See details below for each supported subcommand.
run [--disable-fail-fast --skip-test --stage-list "A10-1, xxx" --gpu-type "A30, H100_PCIe" --add-multi-gpu-test --only-multi-gpu-test --disable-multi-gpu-test --post-merge --extra-stage "H100_PCIe-[Post-Merge]-1, xxx"]Launch build/test pipelines. All previously running jobs will be killed.
--disable-fail-fast(OPTIONAL) : Disable fail fast on build/tests/infra failures.--skip-test(OPTIONAL) : Skip all test stages, but still run build stages, package stages and sanity check stages. Note: Does NOT update GitHub check status.--stage-list "A10-1, xxx"(OPTIONAL) : Only run the specified test stages. Examples: "A10-1, xxx". Note: Does NOT update GitHub check status.--gpu-type "A30, H100_PCIe"(OPTIONAL) : Only run the test stages on the specified GPU types. Examples: "A30, H100_PCIe". Note: Does NOT update GitHub check status.--only-multi-gpu-test(OPTIONAL) : Only run the multi-GPU tests. Note: Does NOT update GitHub check status.--disable-multi-gpu-test(OPTIONAL) : Disable the multi-GPU tests. Note: Does NOT update GitHub check status.--add-multi-gpu-test(OPTIONAL) : Force run the multi-GPU tests. Will also run L0 pre-merge pipeline.--post-merge(OPTIONAL) : Run the L0 post-merge pipeline instead of the ordinary L0 pre-merge pipeline.--extra-stage "H100_PCIe-[Post-Merge]-1, xxx"(OPTIONAL) : Run the ordinary L0 pre-merge pipeline and specified test stages. Examples: --extra-stage "H100_PCIe-[Post-Merge]-1, xxx".For guidance on mapping tests to stage names, see
docs/source/reference/ci-overview.md.kill
killKill all running builds associated with pull request.
skip
skip --comment COMMENTSkip testing for latest commit on pull request.
--comment "Reason for skipping build/test"is required. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break.reuse-pipeline
reuse-pipelineReuse a previous pipeline to validate current commit. This action will also kill all currently running builds associated with the pull request. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break.
Summary by CodeRabbit
Refactor
Chores
Tests