Conversation
Move build declarations, narrow request types, and native encode/embed/rerank handlers into each encoder family. Preserve legacy unsupported-input checks and exercise the owner namespace from existing E2E helpers. Python CPU owner tests passed: 120 passed and 26 unselected E2E cases skipped. Existing model graph definitions and numerical assertions are unchanged. Signed-off-by: yifeif <277870278+yifeif-nv@users.noreply.github.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: NVIDIA/TensorRT-Model-Connect/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (80)
Included review availability: Your plan provides up to 12 included reviews per hour; 4 remain after this review. 📝 SummarySummaryAdds family-owned Each family defines its CLI, build request validation, bundle construction, and native runtime handler. The native handlers reuse the existing task interfaces and check bundle family identity. Build handlers load model code lazily. The reported intent is to preserve existing defaults, inputs, unsupported-input rejection, and flat CLI entry points. The supplied summary reports 296 architecture and CI contract tests, 120 owner CPU tests with 26 selector skips, and 268 integration tests passed with 7 expected no-device skips. It also reports that wheel and installed-package validators passed. Fresh-checkpoint GPU inference, numerical and performance qualification, and TensorRT-RTX execution remain outstanding. These results were not independently verified here. Architecture impact
HUMAN REVIEW REQUIRED — the reported test and packaging results are not independently verified, and the unavailable diff leaves the full change scope unconfirmed. Review severity counts are unavailable because no current review findings were supplied. WalkthroughThis PR adds family-specific CLI build and native runtime commands for ten model families. It moves build request coercion into each family CLI module, updates model builders and E2E calls to use that path, adds native runtime adapters, and adds Python and C++ CLI tests. ChangesFamily-specific CLI rollout
Priority: ⬇️ Low Estimated code review effort: 4 (Complex) | ~75 minutes Merge Risk: ⚪ Minimal · up to The new family CLI commands preserve optional runtime defaults and have no identified merge-blocking risk. 🚥 Pre-merge checks | ✅ 8 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (8 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 6.49% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 262 functions across 50 files. (30 skipped: 20 unsupported, 10 over the file limit.) Comment |
Background
Follow up #1310 by moving ten existing families' CLI definitions and execution into their owning directories. New options can then be added without changing the public parser or shared build request.
Exit Criteria
Implementation
Families:
albert,convbert,deberta,distilbert,dpr,electra,fnet,modernbert,mpnet,roberta.Adds
build,encode,embedandrerank, lazy Python build handlers and native adapter targets. Existing family runtime interfaces are reused inside owner handlers. Model graphs and build policies remain in their existing owners. Existing flat CLI entry points remain compatible; public Task ABI, bundle format and dependencies are unchanged.Change categories
Validation
Commands and Results
Partial remote CI on
8cb9610d3977ce475b2864c273364a0e7e17c659:Stable Community CI— passed;Dev Community CI— failed;TRTMC Internal CI / Automated premerge gate— passed. Dev GPU validation remains incomplete because of a provisioning infrastructure failure; a normal CI retry is pending.CI_BASE_REF=4b9cc2b0f259e8959e1a5c0e996506e60d7101b5 python3 -m tools.ci pipeline source-quality: passed, including 296 architecture and CI contract tests.Owner CPU suite: 120 passed / 26 existing selector skips:
python3 -m pytest families/albert/tests families/convbert/tests families/deberta/tests families/distilbert/tests families/dpr/tests families/electra/tests families/fnet/tests families/modernbert/tests families/mpnet/tests families/roberta/tests -m "not gpu and not trt" -q -p no:cacheprovidercmake --build /work/build --parallel 8andctest --test-dir /work/build --output-on-failure --label-exclude gpu: a local integration tree containing all five independent migration batches passed 268 tests / 7 expected no-device skips, including this batch's 10 native CLI handler tests.python3 -m build --no-isolation --wheel --outdir /work/wheels -Cbuild-dir=/work/wheel-build .: the same integration tree built a real wheel; archive/installed validators passed for all 128 model families. Installed-only probes passed owner build dispatch for all 50 migrated families, 120 offline help commands and 70 native adapter family guards.Hardware, Environment, and Revisions
Head:
8cb9610d3977ce475b2864c273364a0e7e17c659. Base:4b9cc2b0f259e8959e1a5c0e996506e60d7101b5. Local Linux/aarch64 CPU-only validation used TensorRT 11.1.0.106 and Torch 2.12.0+cu130. Synthetic runtime fixtures and probe bundles were used for CLI integration; the skipped tests retain their existing explicit GPU/E2E selectors.Not Run / Remaining Gaps
Fresh checkpoint GPU inference, numerical/performance qualification and TensorRT-RTX execution were not run locally. The passing remote results above cover their configured checks on this head. Dev GPU validation remains incomplete, so merge readiness is pending.
Contributor Self-Review
Reviewed family ownership, supported parameters/defaults, old Python-call compatibility, native input/output contracts and failure behavior. Publication is gated on an independent review of outgoing code, commit metadata, PR text and CI output paths.
Notes For Future Readers
This is batch 4 of five independent ten-family migrations on the same base; no batch depends on another. Review
cli.json,cli.py, nativecli.cpp, then owner tests. Remaining families can follow the same pattern. Remove the shared legacy CLI/request only after migration is complete.Risk level
Ten families gain a new command path and adapter artifacts. Explicit defaults, strict rejection, owner tests and installed-package checks cover the integration boundary; they do not replace model qualification.