Remove bounding box utils and refactor retinanet #2039
Conversation
|
Not sure what best to do about this test failure. @sineeli do you know the minimal version of Keras we would need for all of this? |
It would require latest release |
There was a problem hiding this comment.
Still a few comments to address. The big one, we need to get rid of the private import of keras for that validation function. Let's remove or duplicate.
We might also want to set things up so the library still works with older keras versions, and throws an error when using an OD model that 3.8 is required. But I can help with that part.
|
Oh yeah make sense, we can only import when we have Keras>=3.8.0 version and then throw error when this doesn't satisfy? I hope this works. |
| @@ -18,6 +15,10 @@ | |||
| RetinaNetObjectDetectorPreprocessor, | |||
| ) | |||
|
|
|||
| # Check if Keras version is greater than or equal to 2.10.0 | |||
| if version.parse(keras.__version__) < version.parse("3.8.0"): | |||
There was a problem hiding this comment.
This is not how we can do this. We need this library to be importable with older versions of Keras. And this file will be imported, and this code block run, on library import!
We need to instead assert the version on usage of symbols we care about.
- Add a function here called
assert_bounding_box_support(). - Call it here inside init, first thing. Also add the same check to anchor generator and box matcher layers.
| # Check if Keras version is greater than or equal to 2.10.0 | ||
| if version.parse(keras.__version__) < version.parse("3.8.0"): | ||
| raise ImportError("Requires 3.8.0 or higher.") | ||
|
|
||
|
|
||
| @keras_hub_export("keras_hub.models.RetinaNetObjectDetector") | ||
| class RetinaNetObjectDetector(ImageObjectDetector): |
There was a problem hiding this comment.
We need to align on names here. Either ImageObjectDetector and RetinaNetImageObjectDetector, or ObjectDetector and RetinaNetObjectDetector. That's our general pattern.
I'd prefer just rename ImageObjectDetector -> ObjectDetector everywhere. cc @divyashreepathihalli
| @@ -10,25 +12,36 @@ class RetinaNetImageConverter(ImageConverter): | |||
|
|
|||
| def __init__( | |||
| self, | |||
| image_size=None, | |||
There was a problem hiding this comment.
This seems suspicious. Why are we removing these args? Keep these unaltered, add bounding_box_format after.
non_max_suppression,anchor_generator, andbox_matcherinto the modeling layers for better integration.