Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MLOps 1.5] Expand the built-ins: CV #862

Merged
merged 7 commits into from
Jan 13, 2023
Merged

Conversation

dbogunowicz
Copy link
Contributor

@dbogunowicz dbogunowicz commented Jan 11, 2023

Feature description

  • Added built-in functions: detected_classes, number_detected_objects, mean_score_per_detection, std_score_per_detection.
  • Removed a few built-ins, simplified a few, and added exhaustive unit tests.
  • Now built-ins either return a single float/int or a dictionary (so that in the future we can easily extract the information from the built-in functions and send them to loggers via FunctionLogger).

Coverage:

As specified in the PRD:

cv_image_width
cv_image_height --> those two functions are covered by "image_shape"

cv_image_mean_pixel_r_channel
cv_image_mean_pixel_g_channel
cv_image_mean_pixel_b_channel --> those three functions are covered by "mean_pixels_per_channel"

cv_image_percent_0_pixels -> "fraction_zeros"

cv_num_classes_predicted - count of the number of unique classes in yolo/yolact -> "detected_classes"
cv_num_bboxes_predicted - count of the number of objects detected in yolo/yolact -> "number_detected_objects"

cv_mean_bbox_score - mean score for a prediction in yolo/yolact
cv_max_bbox_score - mean score for a prediction in yolo/yolact
cv_min_bbox_score - min score for a prediction in yolo/yolact -> covered by  "mean_score_per_detection" and "std_score_per_detection"

@dbogunowicz dbogunowicz marked this pull request as ready for review January 11, 2023 14:54
@corey-nm
Copy link
Contributor

What does a configuration of actually using these look like? If I wanted to use a classes thing and an image thing I'd have to specify something like:

pipeline_outputs.image:
    - std_per_channel
pipeline_outputs.classes:
    - count_num_classes

?

corey-nm
corey-nm previously approved these changes Jan 11, 2023
Copy link
Contributor

@corey-nm corey-nm left a comment

Choose a reason for hiding this comment

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

lgtm, would be good to see some frontend examples to see how usable these all are

@dbogunowicz
Copy link
Contributor Author

What does a configuration of actually using these look like? If I wanted to use a classes thing and an image thing I'd have to specify something like:

pipeline_outputs.image:
    - std_per_channel
pipeline_outputs.classes:
    - count_num_classes

?

pipeline_outputs.image:
    - func: std_per_channel
pipeline_outputs.classes:
    - func: count_num_classes

Copy link
Contributor

@bfineran bfineran left a comment

Choose a reason for hiding this comment

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

looks great overall, few comments

src/deepsparse/loggers/metric_functions/cv/__init__.py Outdated Show resolved Hide resolved
src/deepsparse/loggers/metric_functions/cv/built_ins.py Outdated Show resolved Hide resolved
src/deepsparse/loggers/metric_functions/cv/built_ins.py Outdated Show resolved Hide resolved
src/deepsparse/loggers/metric_functions/cv/built_ins.py Outdated Show resolved Hide resolved
src/deepsparse/loggers/metric_functions/cv/built_ins.py Outdated Show resolved Hide resolved
src/deepsparse/loggers/metric_functions/cv/built_ins.py Outdated Show resolved Hide resolved
src/deepsparse/loggers/metric_functions/cv/built_ins.py Outdated Show resolved Hide resolved
src/deepsparse/loggers/metric_functions/cv/built_ins.py Outdated Show resolved Hide resolved
src/deepsparse/loggers/metric_functions/cv/built_ins.py Outdated Show resolved Hide resolved
src/deepsparse/loggers/metric_functions/cv/built_ins.py Outdated Show resolved Hide resolved
dbogunowicz and others added 4 commits January 12, 2023 09:15
Co-authored-by: Benjamin Fineran <bfineran@users.noreply.github.com>
Co-authored-by: Benjamin Fineran <bfineran@users.noreply.github.com>
Copy link
Contributor

@corey-nm corey-nm left a comment

Choose a reason for hiding this comment

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

lgtm!

@dbogunowicz dbogunowicz merged commit ba5ce84 into main Jan 13, 2023
@dbogunowicz dbogunowicz deleted the feature/damian/cv_builtins branch January 13, 2023 15:48
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