Repository navigation
[None][feat] Add get_env_flag and read on/off env switches with it - #21212
yijingl-nvidia wants to merge 9 commits into
Conversation
a0a3c93 to
23046d3
Compare
|
/bot run |
|
PR_Github #81726 [ run ] triggered by Bot. Commit: |
|
PR_Github #81726 [ run ] completed with state
|
|
PR_Github #82881 [ run ] triggered by Bot. Commit: |
|
PR_Github/21212-edcdd68 #82879 was force-killed by a newer pipeline run. |
sunnyqgg
left a comment
There was a problem hiding this comment.
The Blockers from my review of 2026-11-05 are resolved or answered at f2c706d:
test_megamoe_shared_fc12_autotune.py:476now passesget_env_flaginto the exec namespace for_prime_cached_tactics(5ad8f26; import moved to :22 in dfb8714).- Switches with C++ or script readers (FT mode, DISABLE_MPI, USE_*_KVCACHE, GEN_ONLY, LAYERWISE, transfer overlap, skip-softmax stat) keep their exact-"1" reads (3011446), and no switch read with
get_env_flagat H has a native or script reader. serve/render/resources.py:31now readsDISABLE_HARMONY_ADAPTERwithget_env_flaglike the server and router, and the router test coverstrue(5158365).TRTLLM_GVR_TIERS_DISABLEis back on its documented parse (anything but unset/""/"0" engages it) atgvr_topk_decode_dispatch.py:102(3011446).prims_ts_precompile.py:110keeps FlashInfer's exact-"1" check forFLASHINFER_CUTE_DSL_DISABLE_CACHE(3011446).- All Python
TRTLLM_ENABLE_PDLreads now go through_utils.get_env_enable_pdl()(on unless exactly0), matchinggetEnvEnablePDLand the GLM-5.2 TP8 csrc ops (edcdd68).
No CI result on f2c706d yet: PR_Github #82881 had not reported, and the runs on the intermediate commits were aborted or force-killed. The autotuner test fix and the CPU and multi-GPU stages are verified only by reading the code.
No one else reviewed edcdd68 (PDL and MNNVL helpers, 54 files) or f2c706d (local media-path plumbing); they came after the last CodeRabbit pass, and I checked only the parts tied to the Blockers.
get_env_flag reads an on/off environment variable the same way everywhere. Unset or blank gives the caller's default, 1/true/yes/on and 0/false/no/off (any case, surrounding whitespace ignored) give True and False, and any other value raises a ValueError that names the variable. A typo then fails loudly instead of silently selecting the other code path. The Kimi K3 merge-back PRs that follow read their switches with it. Signed-off-by: Harshil Vagadia <hvagadia@nvidia.com> Signed-off-by: Yijing Li <257409031+yijingl-nvidia@users.noreply.github.com>
Replace hand-written on/off env reads such as os.environ.get(X, "0") == "1" with get_env_flag(X, False). Each call keeps the old result when the variable is unset. Only modules that already import tensorrt_llm._utils change, so no import time is added. Switches whose tests require exactly "1" to turn them on keep their current parsing: FORCE_*DETERMINISTIC, TLLM_CUDA_GRAPH_STRICT_BUFFERS, TLLM_EVAL_SPEC_STATS, TRTLLM_DISABLE_FUSED_DSA_METADATA, TRTLLM_GPU_KEEPALIVE, TRTLLM_MEDIA_ALLOW_PRIVATE_URLS and TRTLLM_USE_GDN_REPLAY. Signed-off-by: Yijing Li <257409031+yijingl-nvidia@users.noreply.github.com>
…Python Restore the original reads of switches that C++ code, a shell script or another library also reads, that sit in vendored code, that are first read after the server is ready, or that are read inside code that turns any error into a fallback. For these, get_env_flag would change only one of the readers, fail a running server, or have its ValueError swallowed. Signed-off-by: Yijing Li <257409031+yijingl-nvidia@users.noreply.github.com>
- get_env_flag: overloads so a bool default returns bool, and a Google-style docstring. - Read KIMI_K3_ROUTER_BF16 and the FP4 MLA switches with get_env_flag, so they parse the same way as the switches next to them. - Document the convention in CODING_GUIDELINES.md and AGENTS.md, and add a release note. - Fix texts that still state the old parsing. Signed-off-by: Yijing Li <257409031+yijingl-nvidia@users.noreply.github.com>
…v_flag - DISABLE_HARMONY_ADAPTER: all three readers use get_env_flag, so the workers, the router and the standalone renderer agree on true/yes/on. - TRTLLM_KIMI_PARAM_POLICY, TRTLLM_KIMI_K3_STRICT_TOOL_GRAMMAR and TRTLLM_ALLOW_UNUSED_CHAT_TEMPLATE_KWARGS are read once when their modules load at server start, so a bad value fails the launch instead of every request. - PyExecutor reads TLLM_METRICS_ALL_RANKS once in _initialize_execution. Signed-off-by: Yijing Li <257409031+yijingl-nvidia@users.noreply.github.com>
- TRTLLM_DISALLOW_LOCAL_MEDIA_PATH, TRTLLM_ENABLE_PYAV, TLLM_VISUALGEN_CPU_RNG, TLLM_VISUAL_GEN_LOG_CUDA_MEMORY, TRTLLM_PP_REQ_SEND_ASYNC and the two refit switches are read once at import with get_env_flag, so a bad value fails the launch. - The DeepSeek-V4 per-step switches and TLLM_DISABLE_ALLREDUCE_AUTOTUNE are read once at import instead of on every forward. - TLLM_PRIMTS_WARM_STRICT is read before the precompile's error handlers. - _bootstrap re-raises a ValueError unchanged instead of asking for a rebuild. - Add a test for the NeMo-Skills sandbox remote-host guard. Signed-off-by: Yijing Li <257409031+yijingl-nvidia@users.noreply.github.com>
… helper each - get_env_enable_pdl() moves to _utils and parses TRTLLM_ENABLE_PDL like the C++ getEnvEnablePDL(): on unless the variable is exactly 0. The 35 Python readers that parsed it three different ways now call it, so Python and native kernels agree. It still reads the variable on each call, because env_overrides set it after import. - The eight copies of the MNNVL fabric-handle check call use_mnnvl_fabric_handle(). - Smaller duplicates call their shared helpers: nvtx_debug_enabled(), _is_mm_disagg() and use_py_mamba_cache_manager(). Signed-off-by: Yijing Li <257409031+yijingl-nvidia@users.noreply.github.com>
OpenAIServer reads TRTLLM_DISALLOW_LOCAL_MEDIA_PATH once at construction and passes it to parse_visual_gen_params, so a value an embedding application sets after import still takes effect, and a typo still fails the launch. The PyAV and chat-template error messages now say to set their variables before the process starts, and the sandbox guard test covers an explicit false and runs in the CPU stage. Signed-off-by: Yijing Li <257409031+yijingl-nvidia@users.noreply.github.com>
f2c706d to
070b062
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #82908 [ run ] triggered by Bot. Commit: |
|
All review comments so far are addressed: each of the 56 review threads has a reply and is resolved. The current head, 070b062, is rebased on main, and CI is running on it. |
|
PR_Github #82881 [ run ] completed with state |
|
PR_Github #82908 [ run ] completed with state
|
…cs flag in a test mypy rejects `from tensorrt_llm._torch.flashinfer_utils import get_env_enable_pdl` in the sampler ops because flashinfer_utils re-exported it implicitly. The executor-loop transcript test builds PyExecutor without __init__, so it now sets _metrics_all_ranks, which PyExecutor sets at startup. Signed-off-by: Yijing Li <257409031+yijingl-nvidia@users.noreply.github.com>
|
/bot run --disable-fail-fast |
|
PR_Github #82997 [ run ] triggered by Bot. Commit: |
erictsai-nv
left a comment
There was a problem hiding this comment.
modeling_v2 changes LGTM; they are mechanical and the unset path is unchanged. Nit: qwen4_exp still parses TRTLLM_MODELING_V2_PLE_HOST_OFFLOAD and TRTLLM_MODELING_V2_MOE_PREFILL_COPY by hand.
|
PR_Github #82997 [ run ] completed with state
|
Description
This PR adds
get_env_flag()totensorrt_llm/_utils.pyand uses it for most on/off environment switches that only TensorRT-LLM's Python code reads.Before this PR, each module parsed its switches by hand. Most turned on only for the exact string
1, soX=truemeant off. Some used!= "0", soX=falsemeant on. A typo silently picked one of the two paths. Some switches were read again for every request or step, so a typo surfaced only after the server was up.get_env_flag(name, default)reads a switch the same way wherever it is used:default.1/true/yes/onreturn True, and0/false/no/offreturn False, in any case.ValueErrorthat names the variable.The PR reads 183 switches in 122 files with
get_env_flag. Every changed module already importstensorrt_llm._utilswhen it loads, so this adds no import time. A switch is converted only when all of these hold:ValueError.Switches that used to be read per request, per executor iteration, per refit or per forward step are now read once at startup, so a bad value stops the launch:
TRTLLM_KIMI_K3_STRICT_TOOL_GRAMMAR,TRTLLM_ALLOW_UNUSED_CHAT_TEMPLATE_KWARGS,TRTLLM_ENABLE_PYAV,TLLM_VISUALGEN_CPU_RNG,TLLM_VISUAL_GEN_LOG_CUDA_MEMORY,TRTLLM_PP_REQ_SEND_ASYNC,TLLM_REFIT_DEDUP_MODULES,TLLM_REFIT_SKIP_WRAPPER_CHECK, the DeepSeek-V4 per-step switches andTLLM_DISABLE_ALLREDUCE_AUTOTUNE.TRTLLM_KIMI_PARAM_POLICY.OpenAIServeris constructed:TRTLLM_DISALLOW_LOCAL_MEDIA_PATH, which it passes toparse_visual_gen_params, so a value an embedding application sets after import still applies.PyExecutor._initialize_execution:TLLM_METRICS_ALL_RANKS.DISABLE_HARMONY_ADAPTERis converted in all three of its readers, so the workers, the disaggregated router and the standalone renderer agree. The router reads it per request, but the workers read it at startup, so a bad value stops the launch.What changes for someone who sets a converted switch:
true,yesandonnow turn it on. ForTRTLLM_DISALLOW_LOCAL_MEDIA_PATH,trueused to log a warning and keep server-side paths allowed; it now refuses them.false,noandoffnow turn it off. These switches used to treatfalseas on:TRTLLM_<MODEL>_EAGER_FUSION_DISABLEDfor DeepSeek, GLM, Llama, MiniMax-M3 and Qwen3 (falsedisabled the fusion),TRTLLM_FP4_MLA_FOLD_Q,TLLM_CUDA_OBJECT_COLLECTIVES,TRTLLM_LATE_COMPILE_DETECT,TRTLLM_USE_MAMBA_REPLAY,TRTLLM_RECLAIM_WORKSPACE,TRTLLM_DEEP_GEMM_CACHE_PER_PROCESS,TLLM_LLMAPI_ZMQ_DEBUGandTLLM_LLMAPI_ZMQ_PAIR. TheTRTLLM_V41_*replay switches,TRTLLM_DWDP_PREFETCH_GATHERandTRTLLM_LTX2_AUDIO_CONDITIONAL_SHARDalready treatedfalseas off, but notnooroff.env_overrides, which workers apply after import (its documentation notes this).TRTLLM_ENABLE_PDL, which 128 shipped configs set throughenv_overrides, is still read on each call.Reads that keep their current parsing
TLLM_DISABLE_MPI,TLLM_FAULT_TOLERANCE_MODE,TLLM_USE_FINE_GRAINED_SYNC,TRTLLM_DISABLE_KV_CACHE_TRANSFER_OVERLAP,TRTLLM_DISAGG_BENCHMARK_GEN_ONLY,TRTLLM_DISAGG_LAYERWISE,TRTLLM_PRINT_SKIP_SOFTMAX_STAT,TRTLLM_USE_{NIXL,UCX,MOONCAKE,MPI}_KVCACHEandFORCE_DETERMINISTIC.FLASHINFER_CUTE_DSL_DISABLE_CACHE, theFLA_*variables inmodules/fla/, andMEGA_DIAG_SKIP_TOKEN_BACKincutedsl_megamoe/kernel_src.TRTLLM_MOE_POST_QUANT_ALLTOALLV,TRTLLM_DEEP_EP_DISABLE_P2P_FOR_LOW_LATENCY_MODE,TRTLLM_FORCE_MNNVL_ARandTLLM_AUTOTUNER_DISABLE_SHORT_PROFILE.1to turn them on:FORCE_MOE_KERNEL_DETERMINISTIC,FORCE_ALL_REDUCE_DETERMINISTIC,TLLM_CUDA_GRAPH_STRICT_BUFFERS,TLLM_EVAL_SPEC_STATS,TRTLLM_DISABLE_FUSED_DSA_METADATA,TRTLLM_GPU_KEEPALIVE,TRTLLM_MEDIA_ALLOW_PRIVATE_URLSandTRTLLM_USE_GDN_REPLAY.0turns them on) andTRTLLM_ENABLE_KV_TRANSFER_LIFECYCLE_ENFORCEMENT(accepts only0or1).tensorrt_llm._utils(_bootstrap.py,usage/usage_lib.py), and four reads inside CuTe DSL kernel functions.Other changes:
get_env_enable_pdl()moves to_utils.py(still importable fromflashinfer_utils) and parsesTRTLLM_ENABLE_PDLlike the C++getEnvEnablePDL(): on unless the variable is exactly0. All 35 Python readers call it, so Python and native kernels agree. Before, most Python readers turned PDL off for any value other than1(such astrue), and the GLM-5.2 TEP4 MoE kernels turned it off forfalse,noandoff.use_mnnvl_fabric_handle().nvtx_debug_enabled()for the two NVTX debug switches,_is_mm_disagg()anduse_py_mamba_cache_manager()._bootstrap.pyre-raises aValueErrorraised while the custom ops import, so a bad switch shows its own message instead of a request to rebuild TensorRT-LLM.CODING_GUIDELINES.mdandAGENTS.mdask new on/off switches to useget_env_flag, and the 1.4 release notes describe the new parsing.llm_args.pychanges only one env read, not the LLM API, so this PR has theapi-compatiblelabel.The Kimi K3 merge-back PRs that follow read their switches with
get_env_flag. The first commit is @hvagadia's fromfeat/k3_26.08.For reviewers of the first round: the commits after the first conversion commit narrow the conversion and address the comments, https://github.com/NVIDIA/TensorRT-LLM/pull/21212/files/d925f23bbc78b631c2d30d05d214e2d73df6bb65..070b06268271ec40c5f4f128b0e2f1ff785093f2
Test Coverage
1 test file added, 14 test files changed:
tests/unittest/utils/test_get_env_flag.py: added, CPU only, listed inl0_cpu.yml.tests/unittest/others/test_nemo_skills_eval.py: new test that the NeMo-Skills sandbox refuses a non-loopback host unlessNEMO_SKILLS_SANDBOX_ALLOW_REMOTEis on.tests/unittest/disaggregated/test_router.py: the router'sDISABLE_HARMONY_ADAPTERtest also runs withtrue.tests/unittest/_torch/modeling_v2/test_modeling_v2_claims.py:get_env_flagjoins the imports that compute nothing, so modeling-v2 targets can read switches with it.tests/unittest/_torch/moe/test_megamoe_shared_fc12_autotune.py: the test that runsAutoTuner._prime_cached_tacticsin its own namespace now gives itget_env_flag.tests/unittest/llmapi/apps/test_kimi_serve_extensions.py,test_chat_template_kwargs_guard.py,test_render_callers.py,test_render_core.py,test_serving_extensions.py,tests/unittest/llmapi/test_kv_hints.py,tests/unittest/grpc/openengine/test_multimodal.py,tests/unittest/_torch/pyexecutor/test_model_loader_compile_guard.py,tests/unittest/_torch/visual_gen/test_visual_gen_utils.pyandtest_trtllm_serve_endpoints.py. The visual-gen test that expectedTRTLLM_DISALLOW_LOCAL_MEDIA_PATH=trueto warn and stay allowed is removed.The converted modules run in the existing pre-merge tests.
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
If PR introduces API changes, an appropriate PR label is added - either
api-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin the PR title.Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.Comprehensible Explanation
This PR adds
get_env_flag(name, default)totensorrt_llm/_utils.py. It is one strict parser for the on/off environment variables ("switches") that TensorRT-LLM's Python code reads.1,true,yesandonturn a switch on, and0,false,noandoffturn it off. Unset or blank gives the default. Any other value raises aValueErrorthat names the variable. 183 existing switches now use it.Before this PR, each module parsed its own switches. Most checked
== "1", sotruemeant off. A few checked!= "0", sofalsemeant on. A typo such asturesilently picked one of the two paths, so a benchmark or a deployment could run with a feature off while its owner thought it was on. The Kimi K3 PRs that port code fromfeat/k3_26.08to main add many new switches, so this PR fixes the parsing first and those PRs read their switches withget_env_flag.A strict parser should fail at launch, before the server takes traffic. Some switches were read per request, per executor iteration, per refit or per forward step, where a typo would fail every request. This PR reads those switches once, at import or when the server or executor starts, so a bad value stops the launch.
Reading at import has one limit.
env_overridesis an LLM API field that sets environment variables in each worker after the worker has imported TensorRT-LLM, so an import-time read misses it. 128 shipped configs setTRTLLM_ENABLE_PDLthroughenv_overrides, soget_env_enable_pdl()still reads it on each call. It now parses the value like the C++getEnvEnablePDL()(on unless the value is exactly0), so kernels launched from Python and from C++ make the same PDL choice.Some switches keep their old parsing:
ValueError.The Description section lists them. New on/off switches should use
get_env_flag, andCODING_GUIDELINES.mdnow says so.🤖 Generated with Claude Code