Fix memory leak in falcon weight loader by kaiyux · Pull Request #8 · NVIDIA/TensorRT-LLM

kaiyux · 2023-10-18T15:57:38Z

No description provided.

# This is the 1st commit message: kernel Signed-off-by: Ubuntu <dafrimi@nvidia.com> wip Signed-off-by: Ubuntu <dafrimi@nvidia.com> remove prints Signed-off-by: Ubuntu <dafrimi@nvidia.com> test pass Signed-off-by: Ubuntu <dafrimi@nvidia.com> test refactor with more use cases Signed-off-by: Ubuntu <dafrimi@nvidia.com> refacor Signed-off-by: Ubuntu <dafrimi@nvidia.com> refacor_2 Signed-off-by: Ubuntu <dafrimi@nvidia.com> add tuner wip Signed-off-by: Ubuntu <dafrimi@nvidia.com> autotuner works Signed-off-by: Ubuntu <dafrimi@nvidia.com> bfloat16 works. moer changes to the thop file Signed-off-by: Ubuntu <dafrimi@nvidia.com> is tune for autotuner is True --> gets real tactics configs Signed-off-by: Ubuntu <dafrimi@nvidia.com> wip Signed-off-by: Ubuntu <dafrimi@nvidia.com> wip Signed-off-by: Ubuntu <dafrimi@nvidia.com> zeros + quant mode is works Signed-off-by: Ubuntu <dafrimi@nvidia.com> act int8 Signed-off-by: Ubuntu <dafrimi@nvidia.com> removed fp8 for now Signed-off-by: Ubuntu <dafrimi@nvidia.com> wip Signed-off-by: Ubuntu <dafrimi@nvidia.com> w4a16 linear module Signed-off-by: Ubuntu <dafrimi@nvidia.com> wip Signed-off-by: Ubuntu <dafrimi@nvidia.com> changed cutalss for sm==89 Signed-off-by: Ubuntu <dafrimi@nvidia.com> wip Signed-off-by: Ubuntu <dafrimi@nvidia.com> test linear work Signed-off-by: Ubuntu <dafrimi@nvidia.com> add license Signed-off-by: Ubuntu <dafrimi@nvidia.com> works! Signed-off-by: Ubuntu <dafrimi@nvidia.com> refactor + linear test pass Signed-off-by: Ubuntu <dafrimi@nvidia.com> preprocess in load weights Signed-off-by: Ubuntu <dafrimi@nvidia.com> wip Signed-off-by: Ubuntu <dafrimi@nvidia.com> wip Signed-off-by: Ubuntu <dafrimi@nvidia.com> wip Signed-off-by: Ubuntu <dafrimi@nvidia.com> wip Signed-off-by: Ubuntu <dafrimi@nvidia.com> refactor + rebase Signed-off-by: Ubuntu <dafrimi@nvidia.com> wip Signed-off-by: Ubuntu <dafrimi@nvidia.com> wip Signed-off-by: Ubuntu <dafrimi@nvidia.com> Blackwell not supported Signed-off-by: Daniel Afrimi <dafrimi@nvidia.com> wip Signed-off-by: Daniel Afrimi <dafrimi@nvidia.com> skip blackwell Signed-off-by: Daniel Afrimi <dafrimi@nvidia.com> wip Signed-off-by: Daniel Afrimi <dafrimi@nvidia.com> works Signed-off-by: Ubuntu <dafrimi@nvidia.com> # This is the commit message NVIDIA#2: rebased Signed-off-by: Ubuntu <dafrimi@nvidia.com> # This is the commit message NVIDIA#3: align with my pld worked version of linear Signed-off-by: Ubuntu <dafrimi@nvidia.com> # This is the commit message NVIDIA#4: wip Signed-off-by: Ubuntu <dafrimi@nvidia.com> # This is the commit message NVIDIA#5: refactor Signed-off-by: Daniel Afrimi <danielafrimi8@gmail.com> # This is the commit message NVIDIA#6: refactor Signed-off-by: Daniel Afrimi <danielafrimi8@gmail.com> # This is the commit message NVIDIA#7: refactor Signed-off-by: Daniel Afrimi <danielafrimi8@gmail.com> # This is the commit message NVIDIA#8: refactor Signed-off-by: Daniel Afrimi <danielafrimi8@gmail.com> # This is the commit message NVIDIA#9: sys path Signed-off-by: Daniel Afrimi <danielafrimi8@gmail.com> # This is the commit message NVIDIA#10: sys path Signed-off-by: Daniel Afrimi <danielafrimi8@gmail.com>

Signed-off-by: Barry Kang <43644113+Barry-Delaney@users.noreply.github.com>

Signed-off-by: Barry Kang <43644113+Barry-Delaney@users.noreply.github.com> Signed-off-by: Fanrong Li <23290157+lfr-0531@users.noreply.github.com>

#8344) Signed-off-by: ziyixiong-nv <219238287+ziyixiong-nv@users.noreply.github.com>

…shes Addresses 2ez4bz review (NVIDIA#8 on PR NVIDIA#12944): earlier in this branch the gate that constructs MultimodalRuntimeData was switched from request.multimodal_hashes to mm_spans is not None. The stated rationale was that spans are the direct signal post-decoupling. Empirical stress test with Qwen2.5-VL-3B on a mixed image+video request (the principal scenario where the two signals diverge via Path B in registry.py:input_processor_wrapper) shows the same ValueError in find_input_mm_embeds regardless of chunked prefill being on or off — the crash is pre-existing in the downstream embed handling, not caused by MultimodalRuntimeData construction. Without an empirical benefit, the gate change adds surface without earning it. Reverting to multimodal_hashes preserves the pre-decoupling construction contract (Path A always emits spans alongside hashes). Kept: - mm_contiguous_spans decoupled from MultimodalInput hashing (lives in py_multimodal_data) - _check_mm_spans_present fail-fast (catches malformed payloads early) The embedding-slice ambiguity in find_input_mm_embeds is orthogonal and tracked separately on this branch. Signed-off-by: venkywonka <23023424+venkywonka@users.noreply.github.com>

…sig NVIDIA#8, Phase 15 The combo (PR NVIDIA#13713 + NVIDIA#13728 fold + MLA port) regressed when applied to rc13: with rc13's default-on block reuse, `_handle_responses`'s early-termination branch and `_end_transfer_and_maybe_terminate` can each refuse termination under the right timing, leaving the request with no cleanup owner. Server hangs on scenarios that succeeded on rc11. Captures this in the investigation report: - 03-defect-class-stack.md: add L10 (redundant block-reuse cleanup mechanism on the disagg path) with code sites, customer-visible symptom, and the latent symptoms the Phase 1 stop-gap leaves open (pin leak on cancel/timeout, PP > 1 disagg without block reuse, eviction race in the unpin → release window, regression risk on adjacent code). - 03-defect-class-stack.md: extend the layer-to-signature mermaid with L10 → sig NVIDIA#8 plus a dotted edge to the latent-symptom set. - 02-failure-signatures.md: add sig NVIDIA#8 (rc13 server hang under disagg + block reuse + in-flight cancel) with full root-cause, short-term stop-gap, and medium-term Phase 2 plan. - 05-investigation-timeline.md: add Phase 15 documenting the rc13 regression discovery, the two competing fix proposals, and the recommended staged plan. - 08-next-steps-and-pr-map.md: split item 2 into "land combo with rc13 stop-gap" and a new item 2a "land Phase 2 of the block-reuse-overlap-scheduler design". - README.md: add the rc13 caveat callout in the status section; update navigation language from L1-L8/seven-sig to L1-L10/eight-sig. Adds incentive cross-reference in the existing design doc: - docs/design/block-reuse-overlap-scheduler/README.md: promote Phase 2 status from "deprioritised" to "load-bearing for stable disagg block reuse" with cross-link to the rc13 evidence. - docs/design/block-reuse-overlap-scheduler/phase2-unify-reuse-mechanisms.md: add an "Empirical confirmation: the rc13 regression" section at the top, framing Phase 2 as the architectural answer the rc13 bug predicted. Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>

…st suite Records the design discussion for the disaggregated cancellation stress-test suite. Targets the weekly stress CI goal in TRTLLM-12648 and serves as the implementation-handover spec for another agent to pick up. Key design decisions captured: - Two 2-hour marathon tests covering the two mainstream (KV-cache × transceiver) configurations: V1+C++ and V2+Python. V2+C++ is invalid by design and not tested. - Both marathons use 3P3D on a single 8-GPU node (6 workers @ TP=1). 3P3D provides redundancy for SIGKILL injection and exercises multi-pair coordination (the L10 dual-cleanup-path scenario behind sig NVIDIA#8). - Build on existing infrastructure (setup_disagg_cluster, run_cancel_stress_test, YAML config schema in tests/integration/defs/disaggregated/) rather than duplicate. - Multi-node deferred — the regression class fires single-host; cross-node testing is a deployment-infra concern, not a cancellation correctness concern. - Other modes (1P1D, 4P2D), other combinations (V1+Python, UCX, block reuse off, aggressive timeout) are deferred as parametric follow-ups — the harness is config-driven so each follow-up is a new YAML, not new Python. - Threaded harness (load + canary + injector + log scanner + metrics scraper), not async, to keep failure-isolated and matching the inherently subprocess / filesystem nature of the injector and log scanner. - Canary client with token-equivalent check vs precomputed greedy-decode references catches silent UAF / corruption. Phase 0 doc covers: goal, background, existing infrastructure inventory, deltas to implement, suite specification, workload + injection schedule, pass criteria, YAML schema, file layout for new code (under tests/integration/defs/stress_test/disagg_cancel/), harness architecture, deferred work list, open questions for the implementer, and acceptance criteria for the implementation PR. Top-level README.md updated to link Phase 0 as ready-for-implementation and to mark Phases 1-2 as still to-be-authored. Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>

…view Apply 5 review-driven edits to §16 to address residual concerns: Walk ordering (concern NVIDIA#8): change GMS RO and MX-receiver alias walks from per-module to top-level model.setup_aliases(). Matches the §7 mitigation contract from ai-dynamo/dynamo PR NVIDIA#7053 ("Call model.post_load_weights() (top-level only) before materialize_module_from_gms()"). transform_weights() and cache_derived_state() walks remain per-module since those bodies live on submodules. New "Why setup_aliases() is top-level-only" callout documents the asymmetry. Lifecycle of _weights_transformed (concern #4): new subsection specifying explicit set/reset/orthogonality rules. Includes a 2x2 truth table showing _weights_removed and _weights_transformed track different lifecycles and can take any combination. Reset is the orchestrator's responsibility (e.g., ModelLoader.reload() resets the flag before re-binding tensors); subclasses do not manage reset. Hard preconditions (concern #5): promote MX source-identity completeness from "open question" to "hard precondition P1." Lists transform-affecting parameters that MX identity must cover (attn_backend, quant backend list, FP8/NVFP4 fusion strategy, TP/EP layout, model revision). Specifies an in-tree backend-fingerprint fail-safe as the fallback if upstream MX cannot guarantee completeness. P2 documents that orchestrator owns _weights_transformed reset. Removes redundant open question NVIDIA#6 from the table. Cosmetic fixes (concerns #2, NVIDIA#6): "four stages" -> "three per-module stages plus orchestrator-managed per-process finalization." cache_derived_state description softened to "reserved for data-dependent state where it exists; many existing modules will have empty bodies." Scope clarifications (concerns #1, #3, NVIDIA#7): - Tiny PR scope reframed as "duck-typed helpers, not inheritance" with citations to existing model_loader.py walker pattern. Lists 4 walker helpers (_setup_aliases, _walk_transform, _walk_cache_state, _walk_full_post_load). - Migration callout: when migrating a subclass, the old post_load_weights() override MUST be removed; otherwise the new staged calls silently no-op. - Family PR #2 (Linear/Attention) gains a "quant-method callback decision" note with default = keep quant_method.post_load_weights callback name (no rename). No code changes. Drives Tiny prep PR scope and family-PR migration sequence. References: - TRT-LLM PR NVIDIA#13926 (GMS-only) - TRT-LLM PR NVIDIA#14151 (MX shim refactor) - ai-dynamo/dynamo PR NVIDIA#7053 (upstream GMS prototype) Signed-off-by: Chien-Chun Hung <chienchunh@nvidia.com> Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>

#1 FD <-> chunk position misalignment in P2pHandleExporter Old code kept POSIX FDs in a flat mExportedFds and addressed them in removeHandles() by accumulating chunk counts across earlier pools. Pool-extension paths in exportHandles() append chunks to an existing pool but FDs to the END of mExportedFds, breaking that invariant. Removing a registered range could close another pool's FDs and leak the actual ones, also corrupting any in-flight UDS handshake. Fix: store FDs on P2pMemPool.fds in lockstep with chunks. Removal walks only the owning pool's fds - no cross-pool indexing possible. NVIDIA#2 UDS socket exposed POSIX FDs to other local users /tmp/trt_llm_p2p_fd_*.sock was created with default umask, no peer-credential check. Any same-host process that could connect could SCM_RIGHTS-receive the FDs and import the exporter's GPU memory. Fix: tight umask + explicit chmod 0600 around bind, fail closed if either fails; SO_PEERCRED gate at accept() to refuse non-same-uid clients (defense in depth). NVIDIA#3 P2pTransferContextPool never erased per-thread Contexts Long-lived processes whose callers are transient threads leaked ~64MB cubTempStorage + a worker pool per dead thread; OS thread::id reuse could also hand a fresh thread a stale Context built in a different CUDA context. Fix: pool is now held by shared_ptr (factory + private ctor enforce this); contextForCurrentThread installs a thread_local guard that, on thread exit, calls eraseForCurrentThread via weak_ptr (no-op if pool is already gone, so destruction order between agent and caller threads is safe in either direction). NVIDIA#4 Reads of P2pHandleExporter::mLocalInfo were unprotected getLocalAgentDesc did isSupported() then getLocalInfo().serialize() - a TOCTOU pair where a concurrent registerMemory could append a pool between the two reads, producing a half-written wire blob. Fix: single mInfoMutex protects mLocalInfo / mDetectedHandleType / mUdsPath end-to-end. New serializeIfSupported() does both checks under the lock and replaces the pair at the only caller. The previous mPoolsMutex (added for FDs) is the same mutex, renamed. NVIDIA#8 Document acquire-then-record contract on CudaEventPool Recycled events carry their prior recording; calling cudaEventQuery before record() returns stale state. All current callers already follow acquire->record->query, but the contract is now spelled out on the API doc so future callers don't diverge. Tests: p2pTransferAgentTest +2 regressions - SerializeIfSupportedEmptyOnFreshAgent: locked accessor returns empty on a freshly constructed agent. - TransientCallerThreadsReleaseContextOnExit: 32 transient caller threads each take a Context from the same agent; after join, pool->numContexts() must be exactly 0 (proves thread-exit guard fires). Local validation: p2pMemInfoTest 11/11 transferAgentTest 18/18 (AgentDesc + VmmDescSplitter) p2pTransferAgentTest 36/36 nixlP2pE2ETest 7/7 (+2 env-gated, validated separately) Signed-off-by: Chuang Zhu <111838961+chuangz0@users.noreply.github.com>

…E. All 8 acceptance criteria hold at runtime against... QA verdict: APPROVE (weighted_score 8.6) Change vs main: 7 files changed, 1376 insertions(+), 8 deletions(-) Build iterations run: 13 Code-quality verification: clean after 2 round(s) - round 1 (REJECT): ROUND MIS-SCOPED — patch not actually reviewed. This round's /simplify, checklist, and /code-review executed with cwd = the agent-flow framework repo (branch kleinc/code_quality_improvement_v1) and reviewed the code-quality-stage feature itself, NOT this task's PR patch. The ChatGLM3 code is on branch agent-team/chatglm3-6b-bringup-v1 in the TensorRT-LLM snapshot repo (a different directory), so `/simplify`/`/code-review` (which diff the cwd's HEAD) never touched it. Root cause: the Quality agent runs in the framework repo's cwd, not the snapshot repo; another round will misfire identically unless the Quality agent runs with the TensorRT-LLM task branch as its working directory. I re-reviewed the ACTUAL patch (git diff main in TensorRT-LLM). Real, unaddressed findings: - TESTS (duplication): `_patch_chatglm3_hf_tied_compat()` duplicated in all 4 new test files; `_find_cuda_graph_runner()` and `_assert_cuda_graph_hard_path()` duplicated across test_chatglm3_gsm8k.py and test_chatglm3_replay.py; CHATGLM3_CKPT const, skip decorators, and RuntimeCfg class repeated. Consolidate to a shared util/conftest (keep all coverage). - modeling_chatglm.py (simplification): redundant post-normalization reads of attention_bias/mlp_bias/head_dim via `getattr(...) or ...` after normalize_chatglm_config already set them; normalize_chatglm_config returns config but both callers discard it; a few verbose comments. - ALTITUDE (larger, behavior-sensitive; flagged not applied): custom imperative load_weights vs framework BaseWeightMapper; ChatGLM-specific normalization special-cased in shared config_utils.py; partial_rotary_factor/rope_fusion hardcoded. No fixes applied: the round did not validly review this patch, and edits to modeling/test code cannot be validated against the GSM8K accuracy criterion in this environment (risking a criterion regression). Recommend fixing the Quality agent's working directory (must be the snapshot repo/task branch) before the next round, then applying the test-dedup and post-normalization simplifications above. - round 2 (REJECT): Round 2 finally assessed the ACTUAL task patch (git diff main...agent-team/chatglm3-6b-bringup-v1 in the TensorRT-LLM repo), unlike round 1 whose skill steps mis-scoped to the agent-flow framework repo (cwd). Note: the automated stage still mis-scopes — /simplify and /code-review diff the cwd (agent-flow), not this task's snapshot repo — so I targeted the correct repo manually. /code-review (correctness): CLEAN — returned []. No correctness bugs survive verification. The high-risk items are validated: partial-RoPE, QKV/GQA split sizes (32 heads / 2 KV / head_dim 128), and transformer.* weight-name mapping are exercised by the GSM8K-parity acceptance gate QA approved; the fused-residual path is completed by the final self.norm(hidden_states, residual); embedding matches the Qwen sibling and load_weights handles attention-DP (tp_size = 1 if enable_attention_dp else mapping.tp_size). Speculative "other ChatGLM variant" concerns (_set_missing fill-if-None, head_dim None if kv_channels absent, missing dense/output biases) are not reachable for chatglm3-6b and are guarded by strict _take/consumed accounting. /simplify (round 2): applied one behavior-preserving cleanup (dropped the unused `return config` from normalize_chatglm_config, annotation -> None); it was subsequently reverted in the working tree, so the patch is unchanged. OUTSTANDING (why REJECT): the checklist's test item is not satisfied — heavy test-helper duplication across the 4 new test files (`_patch_chatglm3_hf_tied_compat` in all 4; `_find_cuda_graph_runner`/`_assert_cuda_graph_hard_path`/`RuntimeCfg` across gsm8k+replay; repeated CHATGLM3_CKPT/skip decorators), ~100+ duplicated lines to consolidate into a shared util; plus minor post-normalization config-read redundancy in modeling_chatglm.py. NOT applied here because tensorrt_llm is not importable in this environment and there is no GSM8K/test rig, so a test-file move cannot be validated for collection/pass (risking the acceptance criterion), and the working-tree edit I did make was reverted. RECOMMENDATION: this cannot be resolved by another automated round as currently wired — fix the Quality agent's working directory so /simplify and /code-review target the snapshot repo/task branch, then apply the test-helper de-duplication with the test rig available (or the author applies it directly). Correctness and framework-conformance are clean; the residual is a non-behavioral maintainability cleanup. - round 1 (REJECT): Code-quality round 1 for the chatglm3-6b bringup patch (modeling_chatglm.py, config_utils.py, _torch/models/__init__.py + 4 new test files). /simplify applied (safe, behavior-preserving, verified against repo convention): - modeling_chatglm.py: consume the normalized config fields instead of re-deriving booleans inline — `bias=config.attention_bias`, `dense_bias=config.mlp_bias`, GatedMLP `bias=config.mlp_bias`, and `head_dim=config.head_dim` at both sites (attention + load_weights). Confirmed no base infra reads these fields and that the model already hard-depends on normalize_chatglm_config having run; matches Llama/Phi3/Nemotron/Parakeet. - Both integration tests: removed the unreachable BFS-over-object-graph fallback in `_find_cuda_graph_runner`, keeping the deterministic `_executor.engine.model_engine.cuda_graph_runner` chain (verified in source; the runner is at a fixed location in single-process mode and in a subprocess otherwise, where BFS also fails). Codex checklist pass: the working tree carried minor docstring/comment trims in the ChatGLM3 test files (removed helper docstrings on `_patch_chatglm3_hf_tied_compat`, `_pick_metric_key`, `greedy_generate`, and an inline comment); no behavioral change. /code-review (high) found 10 items. Fixed this turn: (NVIDIA#4) `_pick_metric_key` in test_chatglm3_gsm8k.py now raises loudly instead of silently gating on an arbitrary non-exact_match metric — strict hardening, no change to any real gsm8k run. OUTSTANDING (why not auto-fixed): - CORRECTNESS, latent/out-of-target: (NVIDIA#1) load_weights only handles chatglm3-6b's bias layout — it unconditionally fetches the QKV bias and never loads dense/MLP biases, so ChatGLM2/3 variants with add_qkv_bias=False or add_bias_linear=True would KeyError or trip the strict unconsumed-weight ValueError. (NVIDIA#2) normalize_chatglm_config never maps ChatGLM `rope_ratio` to `rope_theta` (get_hf_rope_theta reads config.rope_theta), so chatglm3-6b-32k/128k silently get theta=10000 and degrade at long context. Both are no-ops for base chatglm3-6b (add_qkv_bias=True/add_bias_linear=False/rope_ratio=1) so QA passed; fixing them adds untestable variant paths (no GPU/checkpoint here) and is scope creep on a base bringup — hand to the coder to fix+test or narrow the module's "ChatGLM2/3" claim. - TEST ROBUSTNESS: (NVIDIA#3) the CUDA-graph hard-path tests require TLLM_WORKER_USE_SINGLE_PROCESS=1 but only name it in an assert string and never set it; without it, ENABLED-cfg tests hard-fail on `assert runner is not None` and BASELINE silently skips its checks. Not auto-fixed because gating/skipping would weaken the author's intended hard-path assertion — needs a CI-env or fixture decision. - ALTITUDE/REUSE (larger refactors, unsafe without test execution): (NVIDIA#5) hand-rolled load_weights + consumed-set + ignorable-suffix lists reimplement BaseWeightMapper/register_mapper (cf. Glm4WeightLoader in modeling_glm.py); (NVIDIA#9) normalize special-cased in shared load_pretrained_config + duplicated in __init__ vs a ChatGLMConfigLoader; (NVIDIA#6) `_patch_chatglm3_hf_tied_compat` (4 copies) + RuntimeCfg/_build_llm/_find_cuda_graph_runner/_assert_cuda_graph_hard_path (2 copies) duplicated across two separate, GPU-gated test trees; (NVIDIA#10) _GreedyHFLM._model_generate hand-rolls HF greedy decode. - EFFICIENCY: (NVIDIA#7) test_chatglm3_source_activation_replay reloads the 6B HF model + rebuilds TRT model for both parametrized scenarios though only the trailing cuda_graph block differs (use the _HFRef cache pattern). - SCOPE CREEP: (NVIDIA#8) the diff reflows unrelated bart/minimaxm3/qwen imports (+ a mistral import in config_utils.py); left as-is because formatter ownership is ambiguous (line 33 is 89 chars vs isort/yapf's 80) and reverting risks fighting the pre-commit hook. Rejecting: a substantive edit was made this turn and several actionable findings remain (test env-var robustness is the most important; the two latent correctness gaps and the altitude/reuse refactors should be triaged by the coder). - round 2 (APPROVE): Code-quality round 2 for the chatglm3-6b bringup patch. /simplify (round 2) applied 3 trivial, behavior-neutral cosmetic cleanups in modeling_chatglm.py: inlined the single-use `head_dim` local in ChatGLMAttention (`head_dim=config.head_dim` passed directly to super()), inlined the single-use `head_dim` temp in normalize_chatglm_config, and dropped the noise `: set` annotation on `consumed`. Reuse/efficiency/altitude agents found nothing new safely-actionable and confirmed the two "small fold" hypotheses don't hold (base `skip_modules` matches model module names, not the checkpoint keys `_IGNORABLE_*` filters; the second normalize_chatglm_config call is load-bearing for the direct-ModelConfig test path) — the only remaining altitude/reuse items are large refactors. Codex checklist pass (round 2): no net changes — independent verification confirmed the only diffs since round 1 were the 4 quality edits above/below, all behavior-neutral. /code-review (high) re-surfaced the same stable finding set (the substantive code was unchanged; no coder commits landed between rounds). I acted on the most actionable one this turn: - FIXED (NVIDIA#3, test robustness): the CUDA-graph hard-path tests required TLLM_WORKER_USE_SINGLE_PROCESS=1 (their ENABLED config hard-asserts the in-process runner) but never set it, so they would spuriously fail/silently-skip when run with a checkpoint outside a harness that exports it. Added an autouse `monkeypatch.setenv` fixture to both integration files (test_chatglm3_gsm8k.py, test_chatglm3_replay.py). Verified the env is read at runtime (utils.py:380, os.environ.get inside a function) so the fixture takes effect before _build_llm; the fix aligns each test with its own documented requirement and cannot regress the already-validated path. - Previously fixed (round 1): _pick_metric_key now fails loudly instead of gating on an arbitrary metric; consumed bias/head_dim inline derivation replaced with normalized fields; BFS runner-finder fallback removed. RESIDUAL (documented follow-ups — not safely resolvable by the automated quality loop): - Out-of-target latent correctness: (NVIDIA#1) load_weights only handles the chatglm3-6b/chatglm2-6b bias layout (add_qkv_bias=True/add_bias_linear=False) — it unconditionally fetches the QKV bias and never loads dense/MLP biases, so ChatGLM variants with add_qkv_bias=False (KeyError) or add_bias_linear=True (unloaded bias + unconsumed-weight ValueError) fail; (NVIDIA#2) rope_ratio is never mapped to rope_theta, so long-context chatglm3-6b-32k/128k (rope_ratio>1) silently use theta=10000. Both are no-ops for the bringup target (base chatglm3-6b/chatglm2-6b, rope_ratio=1); fixing needs variant checkpoints to test and expands scope beyond the bringup — for the coder/a follow-up. - Larger altitude/reuse/efficiency refactors needing test-execution access: (NVIDIA#4/NVIDIA#7) adopt BaseWeightMapper/register_mapper for load_weights + a ChatGLMConfigLoader/config-subclass to remove the shared-runtime normalize special-case and its second call; (NVIDIA#5) consolidate the 4x/2x-duplicated test helpers across the two GPU-gated test trees; (NVIDIA#6) cache the 6B HF/TRT model in the parametrized attention test; (NVIDIA#8) the _GreedyHFLM hand-rolled reference decode. APPROVE rationale: base chatglm3-6b is fully correct (QA-approved, acceptance criteria met); two thorough rounds applied all safe in-scope polish culminating in the NVIDIA#3 test-robustness fix; the quality loop has converged (round 2 reproduced round 1's findings with no coder changes); and every remaining finding is either out-of-target latent correctness for untested ChatGLM variants or a larger refactor requiring test-execution access — none of which another automated /simplify→/code-review round can safely resolve. Residuals are surfaced above for human/coder triage. QA report: DECISION: APPROVE. All 8 acceptance criteria hold at runtime against the current committed code (HEAD 2323424, iter11). Reference is the checkpoint's own trust_remote_code modeling_chatglm.py — an independent HF reference sharing zero code with the TRT-LLM path. PER-CRITERION (runtime evidence): 1. Bootstrap chain — PASS. slurm/core_gpu_tests.4147161.log: "Built target build_wheel_targets", "Successfully installed tensorrt_llm-1.3.0rc21 transformers-5.5.4" (pip install -e .[devel]), "Claude Code successfully installed!", "claude-agent-sdk 0.2.93"; all inside srun pyxis container with --container-mounts=<repo>:<repo> --container-workdir=<repo> under set -eo pipefail (tests ran only because bootstrap rc=0). 2. config_registration_and_weight_accounting — PASS (TIER1 rc=0). Real ckpt loads unedited, arch resolves to ChatGLMForCausalLM, TRTLLM backend + KVCacheManagerV2, strict state-dict accounting (raises on missing/unexpected/shape-mismatch; only rotary_pos_emb.inv_freq tolerated). 3. source_activation_replay — PASS both graph modes. layer0 cosine=1.000000 mean_abs=0.00000, layer27 cosine=0.999999; decode graph-vs-eager max_abs=0.0 cosine=1.0. Companion partial_rope_boundary_and_theta PASS (dims [0:64] rotate, [64:128] pass-through, theta=10000^(-i/32)). 4. source_logit_replay — PASS both configs. trt_tok==hf_tok (5231/30910/4802), cosine~0.99999; enabled num_captured_graphs=8, baseline 0. 5. generation_parity — PASS both configs. 5 prompts x 32 steps, token_mismatches=0, min per-step cosine~0.99998. 6. llm_api_smoke — PASS both configs (nonempty deterministic; V2 + TRTLLM asserted; enabled enabled=True graphs=8, baseline graphs=0). 7. gsm8k accuracy_canary — PASS (slurm/gsm8k_gate.4145862.log): HF=40.00, baseline=40.00 gap 0.00, enabled=40.00 gap 0.00; enabled num_captured_graphs=32. 8. gsm8k full_trtllm_eval — PASS: HF=52.77, baseline=52.99 gap 0.23, enabled=53.07 gap 0.30 (both < 2.0 tol), enabled captured 32 CUDA graphs; RESULT: PASS. Absolute ~53% matches published ChatGLM3-6B GSM8K, ruling out a shared-bug artifact. PROVENANCE: Core log 4147161 started 05:55, after the 05:33 HEAD commit -> definitively iter11 (criteria 1-6). git diff iter10->iter11 (2d58cc7..2323424) touched ONLY the two test files (additive CUDA-graph hard-path assertions); tensorrt_llm/ is byte-identical. GSM8K log 4145862 prints the iter11-only [cuda_graph_hard_path] output -> it ran the current test file against the unchanged runtime code (live bind-mount imported iter11 mid-bootstrap). Criteria 7-8 are therefore verified against current code. INDEPENDENT RERUN THIS TURN: I resubmitted both jobs. Core (4149172) matched the prior post-commit run and was cancelled as redundant. My GSM8K resubmit (4149173) FAILED in ~1.5 min inside build_wheel.py (FileNotFoundError: inherit_graph_6.md5 in copy_resolving_symlink) — a concurrent-build race from my own simultaneous submission of two bootstrap-heavy jobs on one live repo mount, NOT a ChatGLM defect. Accepted evidence is the sequential post-commit runs + full code/provenance review. RED-TEAM (all addressed): independent HF reference (no shared helpers); partial GPT-J RoPE covered analytically + behaviorally (layer0 exact); CUDA-graph silent fallback ruled out by in-process CUDAGraphRunner introspection (runner.enabled + >=1 torch.cuda.CUDAGraph; baseline 0); KV-cache/mask/scale drift ruled out by 32-step parity (0 mismatches) + GSM8K absolute-score match; transformers 5.x bridges (all_tied_weights_keys={}, max_length, num_hidden_layers) are loader-compat only and do not alter ChatGLM forward semantics. EVALUATION SCORES: - Functionality (2.0): 9 — every criterion passes with exact argmax parity, cosine >0.9999, GSM8K within 0.3 pts of HF on both configs. - Code Quality (1.0): 8 — clean, well-documented 373-line model reusing existing modules; narrow config hook; strict weight accounting; minor incidental import reformatting in __init__.py; a pre-existing (non-diff) ruff E501 at __init__.py:130. - Performance (1.5): 8 — parity-first bring-up (no perf gate in task.yaml); production TRTLLM backend + CUDA graph + overlap scheduler + fused QKV/gate-up + KVCacheManagerV2 all working. - Completeness (1.0): 9 — complete, buildable, no placeholders, full state-dict accounting, both runtime configs. - Technical Sophistication (1.0): 9 — correct unfused partial GPT-J RoPE with TRTLLM backend + CUDA-graph capture, compact MQA KV, config/transformers bridges. Weighted = (9x2.0 + 8x1.0 + 8x1.5 + 9x1.0 + 9x1.0)/6.5 = 56/6.5 = 8.6. STRENGTHS: genuine independent HF reference; real CUDA-graph hard-path proof (not just config flags); strict weight accounting; large GSM8K margin; minimal composable design (no new schemas). WEAKNESSES: multi-GPU/perf/chunked-prefill deferred (out of scope per task.yaml); harness must run the two jobs sequentially to avoid the concurrent-build race; pre-existing E501 worth a one-line fix. RECOMMENDATION: APPROVE — no code defect identified; task.yaml completion_criteria (GSM8K within 2 pts with and without CUDA graph + overlap scheduler) is met on both configs.</summary> </invoke>

…o e2e At large concurrency, e2e exercises the full ctx/gen worker interaction and KV-cache transfer path -- more likely to catch a functional regression than gen_only which only stresses the decode side. Swaps: * gb200 kimi-k25-thinking-fp4 1k1k con4096 dep8: gen_only -> e2e * gb300 glm-5-fp4 1k1k con4096 dep8: gen_only -> e2e Both e2e counterparts already exist as post-merge tests, so this is a 1:1 stage-move (pre_merge/post_merge counts unchanged in both files; no jenkins/L0_Test.groovy testCount edits needed). B200 DSR1 con2048 (test NVIDIA#8) intentionally left as gen_only -- its e2e counterpart is commented out in the test-db and has never run; not enabling as a pre-merge blocker. Signed-off-by: Chenfei Zhang <chenfeiz@nvidia.com>

Fix memory leak in falcon weight loader

bca3edf

juney-nvidia merged commit b4af28c into release/0.5.0 Oct 18, 2023

juney-nvidia deleted the fix/falcon-loader-mem-leak branch October 19, 2023 00:43

abhinav-vimal13 mentioned this pull request Nov 20, 2023

failing engine building of llama-2 on V100 DGX #436

Closed

Nam-ang mentioned this pull request Jan 9, 2024

When run llama2, Caught signal 11 (Segmentation fault) #752

Closed

terryaic mentioned this pull request Mar 10, 2025

unable to run fp4 format using LLM API code, reported error:[TensorRT-LLM Error][CutlassFp4GemmRunner][GEMM Dispatch] Arch unsupported for CUTLASS FP4 GEMM #2869

Closed

4 tasks

poweiw added ggtest and removed ggtest labels Jun 3, 2025

litaotju pushed a commit to litaotju/TensorRT-LLM that referenced this pull request Jul 19, 2025

Add compile for quantization kernels (NVIDIA#8)

15eaace

Signed-off-by: Barry Kang <43644113+Barry-Delaney@users.noreply.github.com>

coderabbitai Bot mentioned this pull request Oct 14, 2025

[https://nvbugs/5534705][fix] Skip unnecessary CUDA graph capture (#8… #8344

Merged

1 task

ziyixiong-nv added a commit that referenced this pull request Oct 16, 2025

[https://nvbugs/5534705][fix] Skip unnecessary CUDA graph capture (#8… (

4ad7ef1

#8344) Signed-off-by: ziyixiong-nv <219238287+ziyixiong-nv@users.noreply.github.com>

justremotephone mentioned this pull request Feb 5, 2026

[Bug]: Random "CUDA error: an illegal instruction was encountered" for gpt-oss 120B at Nvidia Spark. #11313

Closed

5 tasks

thorjohnsen mentioned this pull request Mar 16, 2026

[TRTLLM-11513][fix] eliminate dangling reference and TOCTOU races in KVCacheManager #12251

Closed

3 tasks

xxi-nv mentioned this pull request May 8, 2026

[None][refactor] MoEScheduler split + MegaMoE EPLB / multi-chunk / CI integration #13908

Merged

2 tasks

Wanli-Jiang mentioned this pull request Jul 15, 2026

[TRTLLM-14026][feat] BREAKING: Remove C++ modules for legacy TRT backend #16369

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix memory leak in falcon weight loader#8

Fix memory leak in falcon weight loader#8
juney-nvidia merged 1 commit into
release/0.5.0from
fix/falcon-loader-mem-leak

kaiyux commented Oct 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

kaiyux commented Oct 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants