fix(internvl): flatten decoder bundle config - #1055
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughThe InternVL plugin now exposes nested text-decoder settings as bundle-level overrides. An end-to-end test verifies vocabulary, dimensions, attention geometry, head dimension, and BOS token values. ChangesInternVL bundle configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized InternVL configuration fix has direct regression coverage and restored native/reference generation parity; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Description checkExplanation The description is complete and relevant. It covers the background, root cause, exit criteria, implementation, change category, validation results, environment, remaining gaps, future notes, and risk rationale. It also identifies source PR Comment |
Keep strict shared JSON parsing unchanged and flatten InternVL text_config fields through the family-owned bundle hook. Add a family-owned CPU regression that Community CPU selects before protected model proof. Signed-off-by: yifeif-nv <yifeif-nv@users.noreply.github.com>
eb18153 to
09cdba0
Compare
Run the complete CPU-safe Python and declared CPU CTest inventories on every PR while keeping GPU model proofs selective. Separate CTest ownership from resource labels, correct stale GPU metadata, and add the missing InternVL serialized-config producer/consumer contract. Refs: NVIDIA#1053, NVIDIA#1055 Signed-off-by: yifeif-nv <yifeif-nv@users.noreply.github.com>
Run the complete CPU-safe Python and declared CPU CTest inventories on every PR while keeping GPU model proofs selective. Separate CTest ownership from resource labels, correct stale GPU metadata, and add the missing InternVL serialized-config producer/consumer contract. Refs: NVIDIA#1053, NVIDIA#1055 Signed-off-by: yifeif-nv <yifeif-nv@users.noreply.github.com>
Run the complete CPU-safe Python and declared CPU CTest inventories on every PR while keeping GPU model proofs selective. Separate CTest ownership from resource labels, correct stale GPU metadata, and add family-owned serialized-config producer/consumer contracts for InternVL and LocateAnything. Refs: NVIDIA#1053, NVIDIA#1055 Signed-off-by: yifeif-nv <yifeif-nv@users.noreply.github.com>
Run the complete CPU-safe Python and declared CPU CTest inventories on every PR while keeping GPU model proofs selective. Separate CTest ownership from resource labels, correct stale GPU metadata, and add family-owned serialized-config producer/consumer contracts for composite decoder families exposed by strict JSON parsing. Refs: NVIDIA#1053, NVIDIA#1055 Signed-off-by: yifeif-nv <yifeif-nv@users.noreply.github.com>
Background
The protected premerge run for source PR #1053 exposed an existing InternVL3-2B full-generation regression: native inference emitted
!!!!!!!!!!while the Hugging Face reference emittedWhite. The vision encoder, checkpoint loading, C++ tests, and Python model tests still passed, which localized the failure to decoder runtime configuration rather than model weights or vision preprocessing.RCCA
Impact
Root Cause
nlohmann::jsonparsing.text_configobject.parse_base_config()continued to query only the root object.Why Existing Tests Missed It
parse_base_config()with its nestedtext_configshape.Correction And Prevention
Unit / C++ and Pythonwith no broad fallback tier, so the regression is rejected before protected GPU model proof.Exit Criteria
text_configwithout changing shared JSON behavior.Implementation
InternVLPlugin.get_bundle_config_overrides()to emit the typed decoder geometry and BOS token that InternVL's family-ownedModelConfigalready resolves fromtext_config.test_internvl_family_plugin_weights.pywith a CPU-only composite-config assertion covering 28 layers, hidden size 1536, vocabulary size 151674, GQA geometry, and nested BOS.Change categories
Validation
Commands and Results
python -m pytest tests/e2e/models/internvl/test_internvl_family_plugin_weights.py -q -p no:cacheprovider: passed, 7 tests.python3 tools/test_impact.py --base github/main --json: selected the InternVL family unit inbuilder_tests;fallback_tierswas empty.ruff check --config ruff.toml python/tensorrt_model_connect/families/internvl/plugin.py tests/e2e/models/internvl/test_internvl_family_plugin_weights.py: passed.python -m pytest tests/tools/test_model_plugin_encapsulation_static.py -q -p no:cacheprovider: passed, 158 tests.python tools/legal_headers.py --check: passed with zero findings.python3 -m tools.ci model-proof --model internvl --revision 09cdba012ac2cef8e69e0808a5266dbc55bd103b --suite premerge: passed locally; native and reference full generation both producedWhite.Hardware, Environment, and Revisions
09cdba012ac2cef8e69e0808a5266dbc55bd103b, based directly ongithub/main@8fb1b89d447d12067930d19d1a7d1d1340ffc77f.OpenGVLab/InternVL3-2B-hfsnapshotcb57a075cb75a2e6d1b668b128d48bb00ae321d2.Not Run / Remaining Gaps
Notes For Future Readers
python/tensorrt_model_connect/families/internvl/plugin.pyfirst, then its regression intests/e2e/models/internvl/test_internvl_family_plugin_weights.py.Risk level
Risk rationale: the change is family-local and does not alter shared parsing, public API, ABI, or bundle format. It does change InternVL bundle metadata, with direct Community CPU coverage and restored model parity.