Skip to content

feature / extended onnx inference#84

Merged
felix-schultz merged 5 commits intoTM9657:devfrom
simonjanssen:dev
Jun 13, 2025
Merged

feature / extended onnx inference#84
felix-schultz merged 5 commits intoTM9657:devfrom
simonjanssen:dev

Conversation

@simonjanssen
Copy link
Copy Markdown
Collaborator

@simonjanssen simonjanssen commented Jun 9, 2025

  • more generic model loader pattern for future extensions: auto-detect provider/framework the onnx asset has been generated with
  • support for D-FINE models for object detection
    image

@deepsource-io
Copy link
Copy Markdown

deepsource-io bot commented Jun 9, 2025

Here's the code health analysis summary for commits 3758281..9e60fb1. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource Docker LogoDocker✅ SuccessView Check ↗
DeepSource Rust LogoRust❌ Failure
❗ 15 occurences introduced
🎯 2 occurences resolved
View Check ↗
DeepSource JavaScript LogoJavaScript✅ SuccessView Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

@felix-schultz felix-schultz requested a review from Copilot June 13, 2025 21:13
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a more generic ONNX model loader that auto-detects the provider/framework based on model inputs/outputs and adds support for D-FINE object detection models. It refactors the ONNX loading, detection, and classification nodes into a provider-based architecture and updates the draw_bboxes function to be public with reordered drawing logic.

  • Add determine_provider and determine_input_shape utilities to select between D-FINE, YOLO, and TIMM providers
  • Refactor LoadOnnxNode, ObjectDetectionNode, and ImageClassificationNode to use the new provider pattern
  • Make draw_bboxes public and adjust its drawing order

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/catalog/src/image/annotate/draw_boxes.rs Expose draw_bboxes, reorder drawing steps, simplify label handling
packages/catalog/src/ai/ml/onnx/load.rs Remove old tensor detection helpers, add provider factory
packages/catalog/src/ai/ml/onnx/detect.rs Introduce ObjectDetection trait and Dfine/Yolo impls
packages/catalog/src/ai/ml/onnx/classify.rs Introduce Classification trait and Timm impl; rename Prediction to ClassPrediction
packages/catalog/src/ai/ml/onnx.rs Add Provider enum and update SessionWithMeta
Comments suppressed due to low confidence (3)

packages/catalog/src/image/annotate/draw_boxes.rs:47

  • [nitpick] Change the parameter type from &Vec<BoundingBox> to a slice (&[BoundingBox]) to accept any slice and avoid unnecessary Vec references.
bboxes: &Vec<BoundingBox>,

packages/catalog/src/image/annotate/draw_boxes.rs:61

  • The previous implementation displayed class_name when available; this change always uses the index. Restoring the conditional match on bbox.class_name will preserve user-friendly labels.
let label = format!("class {} ({:.2})", bbox.class_idx, bbox.score);

packages/catalog/src/image/annotate/draw_boxes.rs:62

  • Remove the debug println! or replace it with a proper logging call to avoid flooding stdout in production.
println!("{}", &label);

/// # Classification Prediction
#[derive(Default, Serialize, Deserialize, JsonSchema, Clone, Debug)]
pub struct Prediction {
pub struct ClassPrediction {
Copy link

Copilot AI Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ClassPrediction lacks Serialize, Deserialize, and JsonSchema derives, which will break json!(predictions). Add the same derives as on the original Prediction struct.

Copilot uses AI. Check for mistakes.
@felix-schultz felix-schultz merged commit 4ed1dff into TM9657:dev Jun 13, 2025
2 of 7 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.

3 participants