Closed LukeWood closed 1 year ago
Hi @LukeWood , @sebastian-sz
I think there might be a bug in the unit test: https://github.com/keras-team/keras-cv/blob/66d658ab214eaab9655026688a363c2b4052f916/keras_cv/bounding_box/converters_test.py#L108-L116
In this unit test, the behavior of bounding_box.convert_format
is unusual. The source_box and target_box have a shape of (1, 2, 4)
, while the ragged_images have a shape of (2, (100, 50), (100, 50), 3)
.
_raggify
should be applied to source_box
and target_box
before calling bounding_box.convert_format()
, as shown below:
@parameterized.named_parameters(*test_image_ragged)
def test_converters_ragged_images(self, source, target):
source_box = _raggify(boxes_ragged_images[source])
target_box = _raggify(boxes_ragged_images[target])
self.assertAllClose(
bounding_box.convert_format(
source_box, source=source, target=target, images=ragged_images
),
target_box,
)
After that, we can fix _image_shape
as following:
def _image_shape(images, image_shape, boxes):
if images is None and image_shape is None:
raise RequiresImagesException()
if image_shape is None:
if not isinstance(images, tf.RaggedTensor):
image_shape = tf.shape(images)
height, width = image_shape[1], image_shape[2]
else:
height = tf.reshape(images.row_lengths(), (-1, 1))
width = tf.reshape(
tf.reduce_max(images.row_lengths(axis=2), 1), (-1, 1)
)
# remove the check if isinstance(boxes, tf.RaggedTensor)
height = tf.expand_dims(height, axis=-1)
width = tf.expand_dims(width, axis=-1)
else:
height, width = image_shape[0], image_shape[1]
return tf.cast(height, boxes.dtype), tf.cast(width, boxes.dtype)
All unit tests have passed in my local env (pytest keras_cv/
).
I can open a PR for this.
Ahhh - I think you are right. That makes sense how the test failed to catch the issue.
Currently this breaks when operating on ragged images. This can be see in the
examples/layers/random_shear_demo.py
demo.More info available here: https://github.com/keras-team/keras-cv/pull/1537#issuecomment-1479032369
Perhaps we can unit test this behavior for all of our converters.