keras-team / keras-hub

Pretrained model hub for Keras 3
Apache License 2.0
803 stars 243 forks source link

[Object Detection] Add YOLOv11 Architecture and Presets #1952

Open DavidLandup0 opened 1 month ago

DavidLandup0 commented 1 month ago

Draft PR for transparency.

Done

Planned

Out of Scope

These will be exported as separate tasks (i.e. ImagePoseEstimator, ImageInstanceSegmentor, their respective preprocessors, etc.) in separate PRs.

API Considerations

There will be lots of reusability between YOLOv11 OD, YOLOv11 Pose, etc. Some functions such as the non-max-supression can be wrapped into generic public layers and reused between object detectors. We could benefit from refactoring these into general utils in KerasHub (currently, they belong to models, such as in the case of RetinaNet).

Some YOLO models are consistent with the same architecture but rely on a different config. Enabling v11 will enable v8 as well, for example. These can be handled through presets. We could turn YOLOv11 into a generic YOLO class, which is configurable through presets and layers. This lets us support multiple versions, but also easily port and publish YOLOv{N} and subsequent versions in the future with minimal code changes (i.e. a layer or two + config).

/cc @divyashreepathihalli @mattdangerw @fchollet for API discussions and considerations.

mattdangerw commented 5 days ago

Haven't gone over the code yet, but re the API questions...

We could turn YOLOv11 into a generic YOLO class, which is configurable through presets and layers. This lets us support multiple versions, but also easily port and publish YOLOv{N} and subsequent versions in the future with minimal code changes (i.e. a layer or two + config).

I think this is probably the right call? If we think it will overall reduce the amount of code without becoming a spaghetti of if, sounds like a worthy clean up.

Some functions such as the non-max-supression can be wrapped into generic public layers and reused between object detectors.

In general if we are seeing the same layers being used across different models, and we can write a common one that covers both cases, that's a good time to consider pulling things out as a layer. Adding common routines as a layer will up the requirements a bit (solid testing, good docs with examples, etc.), but that's not an issue just something to keep in mind.

Random and not originating on this PR, but ImageObjectDetector feels a little redundant to me. It's not like there's a text object detector. Can we just name this task ObjectDetector and keep our code a little more concise? @divyashreepathihalli WDYT?

DavidLandup0 commented 4 days ago

Thanks for weighing in @mattdangerw!

If we think it will overall reduce the amount of code without becoming a spaghetti of if, sounds like a worthy clean up.

The original repo is written to allow for this customizability, so in principle, it shouldn't be too hard to do it here. Though, the original repo has a lot of proprietary structures and code which we don't want to port, so we'll have to trim down a lot on the sides.

Random and not originating on this PR, but ImageObjectDetector feels a little redundant to me. It's not like there's a text object detector. Can we just name this task ObjectDetector and keep our code a little more concise?

Agreed. It was named ImageObjectDetector to keep consistent with ImageSegmenter, even though we don't have TextSegmenters either. Unless we refactor that, consistency would probably be preferable?

mattdangerw commented 3 days ago

Agreed. It was named ImageObjectDetector to keep consistent with ImageSegmenter, even though we don't have TextSegmenters either. Unless we refactor that, consistency would probably be preferable?

Yeah, let's keep consistent for now! Leave as is for this PR.