Skip to content

refactor(vision): migrate convolutional classifiers to Task SDK - #1332

Merged
yifeif-nv merged 1 commit into
NVIDIA:mainfrom
yifeif-nv:refactor/task-sdk-vision-convolution
Sep 18, 2026
Merged

yifeif-nv merged 1 commit into
NVIDIA:mainfrom
yifeif-nv:refactor/task-sdk-vision-convolution

Conversation

@yifeif-nv

Copy link
Copy Markdown
Collaborator

Background

Migrate ConvNeXt, DenseNet and GhostNet to the existing image-class-score Task SDK.
These three independent families use the same contract, so they are grouped in
one reviewable migration PR. Every changed file belongs to one of:

  • families/timm_convnext/**
  • families/timm_densenet/**
  • families/timm_ghostnet/**

This follows the shared SDK in #1226 and the merged MobileNetV3 pattern in #1308.
It does not introduce a shared model implementation or a dependency between families.

Exit Criteria

  • Each family implements IModel and IImageToClassScores, with its own bind
    declaration, correct bundle primary Task, and complete owned logits.
  • Preserve each graph, weights, preprocessing, declared cases and original oracle;
    do not replace a family's existing acceptance rule with a weaker common check.
  • Pass local CPU and the selected official single-device E2Es. Current-head remote
    checks remain required before merge; this submission does not claim Pre-merge green.

Implementation

  • Switch support, builder metadata and owned manifests from classification to
    image_to_class_scores; no legacy alias or fallback execution.
  • Return all class logits in checkpoint order. Only checkpoint-provided labels
    and vocabulary identity are published; absent identity stays empty. No hashes
    or invented cross-model IDs.
  • Keep each model's computation and image preprocessing unchanged. Add family-local
    binding, input/output, metadata and result-lifetime CPU contracts.
  • Build public C11/C++17 consumers through the existing family CTest dependency
    mechanism. The existing E2Es call both SDKs with identical decoded RGB pixels,
    compare every score, and read results after model release. Original JPEG CLI
    and reference paths remain; different decoders are not claimed bitwise identical.
  • Reuse the existing family-owned performance declaration/script protocol.
    Preserve entry IDs, workloads, precision, 3/10 sampling and 5% margin.
    No shared header, registry, CLI, benchmark implementation, website or workflow edit.

Change categories

  • Model or runtime behavior
  • Public API
  • ABI
  • Bundle or artifact format
  • Dependencies
  • Documentation only
  • CI or developer tooling

Public C layouts and bundle format 1 are unchanged. Primary Task and family
runtime metadata change, so rebuild existing bundles for these family runtimes.

Validation

Commands and Results

With the existing native dependencies and E2E environment:

  • cmake --build build --target trtmc trtmc_backend_trt trtmc_benchmark_worker test_timm_convnext_task_contract test_timm_convnext_image_preprocess test_timm_densenet_task_contract test_timm_densenet_image_preprocess test_timm_ghostnet_task_contract test_timm_ghostnet_image_preprocess:
    passed, including the six public SDK consumer binaries.
  • ctest --test-dir build --output-on-failure -R '^(timm_convnext|timm_densenet|timm_ghostnet)_(task_contract|image_preprocess)$':
    6/6 CPU tests passed.
  • python -m pytest families/timm_convnext/tests families/timm_densenet/tests families/timm_ghostnet/tests -m 'not gpu and not trt and not e2e' -q:
    97 passed; 3 explicitly unselected E2E cases skipped.
  • python -m pytest families/timm_convnext/tests/test_e2e.py families/timm_densenet/tests/test_e2e.py families/timm_ghostnet/tests/test_e2e.py --e2e-testcase convnext-tiny-in12k-ft-in1k --e2e-testcase densenet121-ra-in1k --e2e-testcase ghostnet-100-in1k -q:
    3 passed (three actual checkpoint cases), including C/C++ calls.
  • python3 -m tools.community_ci source-quality --base 9d65d3bf0022255237df36727a3a2b3f6de3a5d4:
    legal, inventory, complexity, formatting and 212 contract tests passed.
  • git diff --check: passed.

The existing native worker and owned references executed all three original
3-warmup / 10-measurement workloads and passed the unchanged classification
output contracts. Each native/reference result retained all 1000 scores.
This is execution/output evidence, not a latency improvement claim.

Hardware, Environment, and Revisions

  • Base: 9d65d3bf0022255237df36727a3a2b3f6de3a5d4.
  • Tested branch contents: 02af02b1e882372d85a31e9e3be76190b3d400a9.
  • Linux/aarch64, single-device NVIDIA GB300 execution per case, CUDA toolkit
    13.3.33, TensorRT 11.1.0.106, PyTorch 2.12.0+cu130 and timm 1.0.28. Selected
    model cases use their existing FP16 manifest settings and checked-in images.
Selected checkpoint Resolved tested revision
timm/convnext_tiny.in12k_ft_in1k aa096f03029c7f0ec052013f64c819b34f8ad790
timm/densenet121.ra_in1k 92007b6200e0b4a4fe68cb4e3947022a928aaaae
timm/ghostnet_100.in1k e524c2ee5d5a9412f802ce38ff895aba07b30910

Not Run / Remaining Gaps

  • No multi-device execution, full Nightly, complete package qualification or
    all-profile GPU qualification. Unselected cases remain declared.
  • No speedup claim. The existing reference timing policy excludes preprocessing,
    while native public Task timing includes family preprocessing; that scope
    distinction remains documented, not declared equivalent.
  • Remote Pre-merge has not yet passed when this PR is opened.

Contributor Self-Review

  • I have completed a self-review of this change.

Reviewed ownership, retained model behavior, test oracles and public evidence.
Independent reviewers checked the exact changes and publication text.

Notes For Future Readers

This branch starts directly from the shared mainline base, not another migration
batch. The three families do not import one another or MobileNetV3; intentional
local duplication preserves independent ownership. Use the existing
TRTMC_NATIVE_BUILD_DIR for development SDK consumer binaries. Core/backend/family
DSOs upgrade together, and old bundles must be rebuilt. Obsolete shared interfaces
will be removed separately after the remaining families migrate.

Risk level

  • Low
  • Medium
  • High

Three family Task identities and runtime metadata change, but model math and
preprocessing are retained and the selected real-model paths were exercised.

Implement family-owned image class-score Tasks, bindings and complete result metadata. Preserve each model graph, preprocessing and existing oracle while exercising public C/C++ consumers and owned benchmark references. No shared implementation or sibling-family dependency is added.

Signed-off-by: yifeif-nv <277870278+yifeif-nv@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b538af08-4bb8-48b9-a988-8605e963d7ce

📥 Commits

Reviewing files that changed from the base of the PR and between 4b74779 and 02af02b.

📒 Files selected for processing (51)
  • families/timm_convnext/README.md
  • families/timm_convnext/model.py
  • families/timm_convnext/runtime/CMakeLists.txt
  • families/timm_convnext/runtime/pipeline.cpp
  • families/timm_convnext/runtime/pipeline.h
  • families/timm_convnext/runtime/plugin.cpp
  • families/timm_convnext/support.py
  • families/timm_convnext/tests/cpp/test_task_contract.cpp
  • families/timm_convnext/tests/manifests/convnext-tiny-in12k-ft-in1k.json
  • families/timm_convnext/tests/performance.yaml
  • families/timm_convnext/tests/performance_reference.py
  • families/timm_convnext/tests/sdk_consumer.c
  • families/timm_convnext/tests/sdk_consumer.cpp
  • families/timm_convnext/tests/test_e2e.py
  • families/timm_convnext/tests/test_model.py
  • families/timm_convnext/tests/test_performance_reference.py
  • families/timm_convnext/tests/test_support.py
  • families/timm_densenet/README.md
  • families/timm_densenet/model.py
  • families/timm_densenet/runtime/CMakeLists.txt
  • families/timm_densenet/runtime/pipeline.cpp
  • families/timm_densenet/runtime/pipeline.h
  • families/timm_densenet/runtime/plugin.cpp
  • families/timm_densenet/support.py
  • families/timm_densenet/tests/cpp/test_task_contract.cpp
  • families/timm_densenet/tests/manifests/densenet121-ra-in1k.json
  • families/timm_densenet/tests/performance.yaml
  • families/timm_densenet/tests/performance_reference.py
  • families/timm_densenet/tests/sdk_consumer.c
  • families/timm_densenet/tests/sdk_consumer.cpp
  • families/timm_densenet/tests/test_e2e.py
  • families/timm_densenet/tests/test_model.py
  • families/timm_densenet/tests/test_performance_reference.py
  • families/timm_densenet/tests/test_support.py
  • families/timm_ghostnet/README.md
  • families/timm_ghostnet/model.py
  • families/timm_ghostnet/runtime/CMakeLists.txt
  • families/timm_ghostnet/runtime/pipeline.cpp
  • families/timm_ghostnet/runtime/pipeline.h
  • families/timm_ghostnet/runtime/plugin.cpp
  • families/timm_ghostnet/support.py
  • families/timm_ghostnet/tests/cpp/test_task_contract.cpp
  • families/timm_ghostnet/tests/manifests/ghostnet-100-in1k.json
  • families/timm_ghostnet/tests/performance.yaml
  • families/timm_ghostnet/tests/performance_reference.py
  • families/timm_ghostnet/tests/sdk_consumer.c
  • families/timm_ghostnet/tests/sdk_consumer.cpp
  • families/timm_ghostnet/tests/test_e2e.py
  • families/timm_ghostnet/tests/test_model.py
  • families/timm_ghostnet/tests/test_performance_reference.py
  • families/timm_ghostnet/tests/test_support.py

Included review availability: Your plan provides up to 12 included reviews per hour; 7 remain after this review.


📝 Summary

Summary

Migrates timm ConvNeXt, DenseNet, and GhostNet to the image_to_class_scores Task SDK.

Each family now:

  • Implements IModel and IImageToClassScores.
  • Returns complete checkpoint-order logits.
  • Publishes class-count, vocabulary, and label metadata.
  • Validates inputs, configuration, output shape, dtype, and metadata.
  • Adds family-owned contract tests, E2E parity checks, SDK consumers, benchmarks, and documentation.

Bundle format 1 and public C layouts remain unchanged. Existing bundles require rebuilding because task and runtime metadata changed.

Architecture impact

  • Family-owned files: All model, runtime, plugin, support, manifest, test, SDK consumer, benchmark, and documentation changes remain within the three affected family directories.
  • Shared surfaces: The change consumes existing Task SDK, runtime, and public C/C++ SDK contracts. No shared implementation or central registry change is reported.
  • Dependency directions: Family runtimes depend on shared model-agnostic runtime and SDK interfaces. No cross-family dependency is reported.
  • Affected consumers: Bundle builders, runtime loaders, SDK clients, E2E tests, and performance integrations must use image_to_class_scores.
  • Unresolved blast radius: Multi-device execution, full Nightly coverage, complete package qualification, all-profile GPU qualification, and remote Pre-merge checks remain outstanding.

Validation

The supplied results report successful builds, six CPU contract/preprocessing tests, 97 family Python tests, three selected single-device E2E tests, source-quality checks, and git diff --check.

Review finding counts are unavailable from the supplied evidence.

Outcome

HUMAN REVIEW REQUIRED

Walkthrough

Changes

TIMM task contract migration

Layer / File(s) Summary
Bundle metadata and runtime contracts
families/timm_convnext/model.py, families/timm_convnext/runtime/*, families/timm_densenet/model.py, families/timm_densenet/runtime/*, families/timm_ghostnet/model.py, families/timm_ghostnet/runtime/*
ConvNeXt, DenseNet, and GhostNet now use image_to_class_scores. Bundles publish class counts, vocabulary IDs, and labels. Pipelines validate RGB float32 requests and return complete owned logits with metadata.
Contract tests and SDK consumers
families/timm_*/runtime/CMakeLists.txt, families/timm_*/tests/cpp/*, families/timm_*/tests/sdk_consumer.*, families/timm_*/tests/test_e2e.py
CPU contract tests and C/C++ SDK consumers validate preprocessing, metadata, logits, invalid inputs, runtime configuration rejection, and native-output parity.
Performance references and documentation
families/timm_*/tests/performance*, families/timm_*/README.md
Each family adds a timed reference workflow, CPU validation tests, performance-suite configuration, and documentation for the task, input, output, SDK, and timing contracts.

Priority: ➖ Normal

Estimated code review effort: 5 (Critical) | ~90 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant E2ETest
  participant SDKConsumer
  participant TRTMC
  participant TIMMPipeline
  participant TRTModule
  E2ETest->>SDKConsumer: provide RGB float32 image and bundle
  SDKConsumer->>TRTMC: load model and request image_to_class_scores
  TRTMC->>TIMMPipeline: validate request and runtime configuration
  TIMMPipeline->>TRTModule: execute preprocessed NCHW input
  TRTModule-->>TIMMPipeline: return complete logits
  TIMMPipeline-->>TRTMC: return scores and metadata
  TRTMC-->>SDKConsumer: expose result view
  SDKConsumer-->>E2ETest: emit JSON score result
Loading

Suggested reviewers: zhenshanx-nv

Merge Risk: ⚪ Minimal · up to 02af0

The task-contract migration appears mergeable after normal checks, with no identified behavior requiring correction.

🚥 Pre-merge checks | ✅ 7 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 237 functions across 39 files. (12 skipped… Write docstrings for the functions missing them to satisfy the coverage threshold.
Benchmark Validation Integrity ⚠️ Warning The new benchmark pair does not measure equivalent regions. In each added families/timm_{convnext,densenet,ghostnet}/tests/performance_reference.py, image preprocessing and host input transfer occur… Use one timing boundary for all three families. Prefer changing the custom reference to declare task-pipeline-call-wall with input_preparation_included: true and perform the same preprocessing and host input transfer inside every timed …
✅ Passed checks (7 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: migrating vision classifier families to the Task SDK.
Description check ✅ Passed The description follows the required template and covers background, exit criteria, implementation, change categories, validation results, environment, remaining gaps, self-review, and risk.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Family Ownership Boundary ✅ Passed PASS. The authoritative diff changes only families/timm_convnext/**, families/timm_densenet/**, and families/timm_ghostnet/**. The cross-family token scan found no reference from one changed fam…
Shared Semantic Neutrality ✅ Passed PASS. The reviewed range changes 51 files, and every path is under families/timm_convnext/, families/timm_densenet/, or families/timm_ghostnet/. The changes are family-owned Python, runtime, tes…
Shared Change Blast Radius ✅ Passed No shared surface changed. The reviewed diff contains 51 files, all under the three family-owned directories. The new CMake targets, SDK consumers, benchmarks, and contract tests are registered and li…
Full details: Docstring Coverage

Explanation

Docstring coverage is 6.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 237 functions across 39 files. (12 skipped: 12 unsupported.)

Full details: Benchmark Validation Integrity

Explanation

The new benchmark pair does not measure equivalent regions. In each added families/timm_{convnext,densenet,ghostnet}/tests/performance_reference.py, image preprocessing and host input transfer occur in _load_reference before _measure starts, while families/timm_{convnext,densenet,ghostnet}/runtime/pipeline.cpp::run performs family preprocessing inside the timed native task.run call (apps/benchmark/native/benchmark_worker.cpp brackets that call with its clock). The changed READMEs explicitly document this difference. The reference times model execution plus float32 device-to-host logits transfer, and performs argmax, finite checks, and JSON reporting after timing. The native worker also performs reduction and serialization after timing, so those regions align. However, preprocessing remains included only on the native side. The new pipeline also validates logits dtype, shape, and class count inside the timed call, while the reference validates score shape and finiteness only in _summary after timing.

Resolution

Use one timing boundary for all three families. Prefer changing the custom reference to declare task-pipeline-call-wall with input_preparation_included: true and perform the same preprocessing and host input transfer inside every timed invocation, while keeping asset loading outside. Also make output validation equivalent: either validate dtype, shape, class count, and finiteness inside the reference's timed invocation, or move equivalent validation outside the candidate timing boundary. Keep synchronization, device-to-host logits transfer, reduction, and serialization placement identical on both paths.


Comment @coderabbitai help to get the list of available commands.

@yifeif-nv yifeif-nv added the run-internal-ci Maintainer-approved dispatch to internal CI label Sep 17, 2026
@github-actions github-actions Bot removed the run-internal-ci Maintainer-approved dispatch to internal CI label Sep 17, 2026
@yifeif-nv
yifeif-nv merged commit 4089ea9 into NVIDIA:main Sep 18, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant