-
Notifications
You must be signed in to change notification settings - Fork 37
Add configs to detect target pipeline #242
Conversation
maxlou05
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed
config.yaml
Outdated
| model_path: "tests/model_example/yolov8s_ultralytics_pretrained_default.pt" # See autonomy OneDrive for latest model | ||
| save_prefix: "log_comp" | ||
|
|
||
| detect_brightspot: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry just noticed this now, can we move this section under detect_target and call it config, like it is for video_input? In this case, you will comment out this whole section because we are using the ultralytics config
config.yaml
Outdated
| option: 0 # 0 is for Ultralytics and 1 is for brightspot (from detect_target_factory.py) | ||
| save_prefix: "log_comp" | ||
|
|
||
| detect_ultralytics: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move this under detect_target and call it config, like it is for video_input?
main_2024.py
Outdated
| DETECT_TARGET_ULTRALYTICS_DEVICE = ( | ||
| "cpu" if args.cpu else config["detect_ultralytics"]["device"] | ||
| ) | ||
| DETECT_TARGET_ULTRALYTICS_MODEL_PATH = config["detect_ultralytics"]["model_path"] | ||
| DETECT_TARGET_ULTRALYTICS_OVERRIDE_FULL_PRECISION = args.full | ||
|
|
||
| DETECT_TARGET_BRIGHTSPOT_PERCENTILE_THRESHOLD = config["detect_brightspot"][ | ||
| "brightspot_percentile_threshold" | ||
| ] | ||
| DETECT_TARGET_BRIGHTSPOT_FILTER_BY_COLOR = config["detect_brightspot"]["filter_by_color"] | ||
| DETECT_TARGET_BRIGHTSPOT_BLOB_COLOR = config["detect_brightspot"]["blob_color"] | ||
| DETECT_TARGET_BRIGHTSPOT_FILTER_BY_CIRCULARITY = config["detect_brightspot"][ | ||
| "filter_by_circularity" | ||
| ] | ||
| DETECT_TARGET_BRIGHTSPOT_MIN_CIRCULARITY = config["detect_brightspot"]["min_circularity"] | ||
| DETECT_TARGET_BRIGHTSPOT_MAX_CIRCULARITY = config["detect_brightspot"]["max_circularity"] | ||
| DETECT_TARGET_BRIGHTSPOT_FILTER_BY_INERTIA = config["detect_brightspot"][ | ||
| "filter_by_inertia" | ||
| ] | ||
| DETECT_TARGET_BRIGHTSPOT_MIN_INERTIA_RATIO = config["detect_brightspot"][ | ||
| "min_inertia_ratio" | ||
| ] | ||
| DETECT_TARGET_BRIGHTSPOT_MAX_INERTIA_RATIO = config["detect_brightspot"][ | ||
| "max_inertia_ratio" | ||
| ] | ||
| DETECT_TARGET_BRIGHTSPOT_FILTER_BY_CONVEXITY = config["detect_brightspot"][ | ||
| "filter_by_convexity" | ||
| ] | ||
| DETECT_TARGET_BRIGHTSPOT_MIN_CONVEXITY = config["detect_brightspot"]["min_convexity"] | ||
| DETECT_TARGET_BRIGHTSPOT_MAX_CONVEXITY = config["detect_brightspot"]["max_convexity"] | ||
| DETECT_TARGET_BRIGHTSPOT_FILTER_BY_AREA = config["detect_brightspot"]["filter_by_area"] | ||
| DETECT_TARGET_BRIGHTSPOT_MIN_AREA_PIXELS = config["detect_brightspot"]["min_area_pixels"] | ||
| DETECT_TARGET_BRIGHTSPOT_MAX_AREA_PIXELS = config["detect_brightspot"]["max_area_pixels"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of having this many variables, just have one DETECT_TARGET_CONFIG and use match case here (see VIDEO_INPUT_CAMERA_CONFIG above
| config: ( | ||
| detect_target_brightspot.DetectTargetBrightspotConfig | ||
| | detect_target_ultralytics.DetectTargetUltralyticsConfig | ||
| ), | ||
| local_logger: logger.Logger, | ||
| show_annotations: bool, | ||
| save_name: str, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we re-order these so that it goes save_name, show_annotations, config, local_logger?
maxlou05
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (as long as you have sort of tested it?)
* Create ultralytics config class * Add configs to target detection pipeline * Organize YAML and improve parsing * Reorder args * Update integration test
* Create ultralytics config class * Add configs to target detection pipeline * Organize YAML and improve parsing * Reorder args * Update integration test
No description provided.