From 82eafde35fce5aa7cbe57b6af49cb990f5edaf2c Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Wed, 10 Apr 2019 00:43:28 +0000 Subject: [PATCH 1/2] Fix negative axis issue with ragged tensor and reduce_sum This fix tries to address the issue raised in 27497 where `tf.reduce_sum` with multiple negative axes and ragged tensor does not produce correct result. The issue is that during reduce op, ragged tensor will reduce one axis at a time. However, for negative axis, sort result is reversed so order is different. This fix convert to positive before the sort to make sure the order. This fix fixes 27497. Signed-off-by: Yong Tang --- tensorflow/python/ops/ragged/ragged_math_ops.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tensorflow/python/ops/ragged/ragged_math_ops.py b/tensorflow/python/ops/ragged/ragged_math_ops.py index 02e927b6991f8d..fa5b8b28d93f28 100644 --- a/tensorflow/python/ops/ragged/ragged_math_ops.py +++ b/tensorflow/python/ops/ragged/ragged_math_ops.py @@ -461,6 +461,12 @@ def _ragged_reduce_aggregate(reduce_op, elif len(axis) == 1: axis = axis[0] else: + # When reducing multiple axes, as we reduce one at a time (see below), + # the negative axis has to be converted to positive at the first run + # as the sort with negative axis will have different orders. + # See GitHub issue 27497. + axis = [ragged_util.get_positive_axis( + a, rt_input.shape.ndims) for a in axis] # When reducing multiple axes, just reduce one at a time. This is less # efficient, and only works for associative ops. (In particular, it # does not work for reduce_mean.) However, reducing multiple axes at From f303882d7a4f97202a378814aabdcb22d960af58 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Wed, 10 Apr 2019 00:47:29 +0000 Subject: [PATCH 2/2] Add test case for GitHub issue 27497 Signed-off-by: Yong Tang --- .../python/ops/ragged/ragged_reduce_op_test.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tensorflow/python/ops/ragged/ragged_reduce_op_test.py b/tensorflow/python/ops/ragged/ragged_reduce_op_test.py index a9fa378eebc01e..cac2d2d5f53403 100644 --- a/tensorflow/python/ops/ragged/ragged_reduce_op_test.py +++ b/tensorflow/python/ops/ragged/ragged_reduce_op_test.py @@ -304,6 +304,18 @@ class RaggedReduceOpsTest(ragged_test_util.RaggedTensorTestCase, rt_input=[[[1, 2], [3, 4, 5]], [[6, 7], [8]], [[9]]], axis=2, expected=[[mean(1, 2), mean(3, 4, 5)], [mean(6, 7), 8], [9]]), + + # Test case for GitHub issue 27497, multiple negative axes. + dict( + ragged_reduce_op=ragged_math_ops.reduce_sum, + rt_input=[[[1, 2], [], [3, 4, 5]], [[6, 7], [], [8]], [], [[9]]], + axis=[-2, -1], + expected=[1 + 2 + 3 + 4 + 5, 6 + 7 + 8, 0, 9]), + dict( + ragged_reduce_op=ragged_math_ops.reduce_sum, + rt_input=[[[1, 2], [], [3, 4, 5]], [[6, 7], [], [8]], [], [[9]]], + axis=[-3, -2, -1], + expected=sum([1, 2, 3, 4, 5, 6, 7, 8, 9])), ) def testReduce(self, ragged_reduce_op, rt_input, axis, expected): rt_input = ragged_factory_ops.constant(rt_input)