Skip to content

Commit

Permalink
[Test][Topi] Avoid depending on f32 rounding behavior for crop_and_di…
Browse files Browse the repository at this point in the history
…vide tests (#13773)

* [Test][Topi] Use binary fractions for crop_and_divide unit tests

The `crop_and_resize` operator uses floating-point arithmetic to
determine whether an index is within a view-box.  This can cause the
use of `extrapolation_value` to depend on target-dependent rounding
differences.

For example, this issue was initially noticed on Vulkan during
debugging of #13530, and was the
result of computing `0.2*223.0 + 0.8*223.0 < 223.0`.  If all
intermediates are cast to float32, the left-hand side evaluates to
`223.00002`.  If intermediates are kept at a higher precision, the
left-hand side evaluates to `223.0`.

The floating-point indexing can't be removed, because the operator
must match the API defined by TensorFlow's operator implementation.
The TensorFlow documentation for
[`CropAndResize`](https://www.tensorflow.org/api_docs/cc/class/tensorflow/ops/crop-and-resize)
does not specify behavior in these cases, nor do the current
TensorFlow unit tests check cases of rounding error.  Since the
TensorFlow unit tests only use binary fractions for the `boxes`
argument, which largely avoids the rounding issue, this commit updates
the TVM unit tests to avoid depending on floating-point precision.

* Use seeded random data for unit test

* Add epsilon offset to avoid depending on floating-point behavior

* Use randomly-generated boxes for unit tests

This mimics the example usage of `tf.image.crop_and_resize`, whose API
this operator is intended to follow.  Using any boxes with edges
representable as integer fractions has the potential for the in-bounds
check to be impacted by rounding error (e.g. `0.2*x + 0.8*x < x`).
Unfortunately, there's no way to remove this possibility altogether
without changing the API, such as accepting the box location as an
integer fraction, rather than a `float32`, but that would break
compatibility.

To avoid the risk of a flaky unit test based on the specific random
boxes used, the PRNG is seeded prior to generation.
  • Loading branch information
Lunderberg authored Apr 5, 2023
1 parent fd7e364 commit 6caf085
Showing 1 changed file with 6 additions and 2 deletions.
8 changes: 6 additions & 2 deletions tests/python/relay/test_op_level5.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,14 +236,18 @@ def test_crop_and_resize(self, target, dev, executor_kind, layout, interpolate_m

extrapolation_value = 0.0

np.random.seed(0)

eps = 1e-4

if layout == "NHWC":
img_shape = (10, 224, 224, 3)
boxes = np.array([[0.1, 0.2, 0.8, 0.7], [0.2, 0, 1, 0.6]]).astype("float32")
boxes = np.random.uniform(size=(2, 4)).astype("float32")
box_indices = np.array([1, 0]).astype("int32")
crop_size = np.array([20, 30]).astype("int32")
elif layout == "NCHW":
img_shape = (5, 3, 255, 255)
boxes = np.array([[0, 0, 1, 1], [0.2, 0.1, 1, 0.9]]).astype("float32")
boxes = np.random.uniform(size=(2, 4)).astype("float32")
box_indices = np.array([0, 1]).astype("int32")
crop_size = np.array([30, 30]).astype("int32")
else:
Expand Down

0 comments on commit 6caf085

Please sign in to comment.