Conversation
Add family-owned build and classify commands for ConvNeXt, CrossViT, DenseNet, DPN, EfficientNet, GhostNet, HRNet, Inception, Inception-ResNet, and Inception-v4. Keep existing Task SDK or legacy native interfaces and preserve old request rejection rules through owner-local conversions. Move owner E2E invocations to the new command prefixes without changing model math or their assertions. Add CPU coverage for declaration/manifest consumers, bundle publication and native callback execution. Validation: 415 owner CPU tests passed, 11 skipped; 53 architecture checks passed. Ruff, changed-function CCN <= 10, clang-format and diff checks pass. GPU checkpoint validation was not run locally. 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 (95)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 SummarySummaryRefactors ten The change preserves existing defaults, supported inputs, rejection behavior, model graphs, build policies, runtime interfaces, flat CLI compatibility, Task ABI, bundle format, and dependencies. E2E tests now use family-specific command prefixes and build APIs. Architecture impact
ValidationReported validation includes 296 architecture and CI contract tests, 415 owner CPU tests with 11 existing selector skips, native CLI handler tests, wheel and installed-package validation, and owner dispatch, help, and adapter probes. Review finding counts are unavailable from the supplied evidence. Outcome: HUMAN REVIEW REQUIRED WalkthroughAdded family-owned build and classify CLI support for ten Timm model families. The changes add validated build flows, native classification adapters, CMake integration, contract tests, E2E wiring, manifest updates, and family-qualified documentation. ChangesTimm family CLI support
Priority: ➖ Normal Estimated code review effort: 5 (Critical) | ~90 minutes Change: Refactor Merge Risk: ⚪ Minimal · up to No actionable merge-blocking risk remains in the supplied evidence. 🚥 Pre-merge checks | ✅ 8 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (8 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 9.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 272 functions across 50 files. (45 skipped: 35 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:
timm_convnext,timm_crossvit,timm_densenet,timm_dpn,timm_efficientnet,timm_ghostnet,timm_hrnet,timm_inception,timm_inception_resnet,timm_inception_v4.Adds
buildandclassify, 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
Remote CI on
5f2956172db402f1a71db12759dfd1dca1e6b06a:Stable Community CI— passed;Dev Community CI— passed;TRTMC Internal CI / Automated premerge gate— passed.CI_BASE_REF=4b9cc2b0f259e8959e1a5c0e996506e60d7101b5 python3 -m tools.ci pipeline source-quality: passed, including 296 architecture and CI contract tests.Owner CPU suite: 415 passed / 11 existing selector skips:
python3 -m pytest families/timm_convnext/tests families/timm_crossvit/tests families/timm_densenet/tests families/timm_dpn/tests families/timm_efficientnet/tests families/timm_ghostnet/tests families/timm_hrnet/tests families/timm_inception/tests families/timm_inception_resnet/tests families/timm_inception_v4/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:
5f2956172db402f1a71db12759dfd1dca1e6b06a. 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 remote premerge result above covers the configured CI checks for this exact head; it does not establish coverage beyond those checks.
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 1 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.