Skip to content

Adding KvCacheConfig, SchedulerConfig, and DynamicBatchConfig #7576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Adding KvCacheConfig, SchedulerConfig, and DynamicBatchConfig #7576

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions tensorrt_llm/llmapi/llm_args.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2621,6 +2621,9 @@ def update_llm_args_with_extra_dict(
"lora_config": LoraConfig,
"moe_config": MoeConfig,
"attention_dp_config": AttentionDpConfig,
"kv_cache_config": KvCacheConfig,
Copy link
Collaborator

@pcastonguay pcastonguay Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nv-kmcgill53 see comment here: https://github.com/NVIDIA/TensorRT-LLM/pull/5610/files#r2178917332

From what I understand, we should not need to explicitly add those. @Superjomn to confirm.

Copy link
Collaborator

@pcastonguay pcastonguay Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a case where the args are not parsed as expected?

Copy link
Contributor Author

@nv-kmcgill53 nv-kmcgill53 Sep 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do. There is a lot going on in this section of code, but to summarize:

  1. We create a KvCacheConfig object on line 161
  2. We put this object in our arg_map on line 184
  3. If we provide the engine_config.yaml on the command line then we execute line 194, where we call into the trtllm api to parse the yaml file and return to us the updated arg map.
  4. If we don't provide a yaml file then we have the KvCacheConfig option that we originally set in line 161.

At the end of step 3, we end up replacing the KvCacheConfig object with a dictionary. Thus we have an inconsistent api interface with TRTLLM and we get errors on lines 195 and below depending on how our dynamo worker gets called.

While you may be able to give a recommended way to interfacing with TRTLLM, it's not very clear from the documentation why some config options get parsed into their own objects while others live in a dictionary.

Copy link
Collaborator

@pcastonguay pcastonguay Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Superjomn can you elaborate on that? Why were those configs removed from update_llm_args_with_extra_dict?

Copy link
Collaborator

@Superjomn Superjomn Sep 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Superjomn can you elaborate on that? Why were those configs removed from update_llm_args_with_extra_dict?

Those are Pydantic configs, and can be initialized from a dict natively, no need to map with an additional dict.

"scheduler_config": SchedulerConfig,
"dynamic_batch_config": DynamicBatchConfig,
}
for field_name, field_type in field_mapping.items():
if field_name in llm_args_dict:
Expand Down