Fix a couple more bugs, add a bunch of unit tests

src/ngraph/pass/dyn_elimination.cpp

@@ -265,7 +265,8 @@ static SlicePlan make_plan(const Shape& input_shape,
             int64_t min_real_end = (is_reverse ? -1 : 0);
             real_end = std::max(min_real_end, std::min(int64_t(input_shape[i_in]), real_end));
 
-            // Adjust the stride for backwards slicing.
+            // Ensure stride is not zero, and adjust it for backwards slicing.
+            NGRAPH_CHECK(strides[i] != 0);
             int64_t real_stride = std::abs(strides[i]);
 
             // Adjust for reversal if needed. This isn't quite as simple as swapping begin and
@@ -281,6 +282,13 @@ static SlicePlan make_plan(const Shape& input_shape,
                 p.reverse_axes.insert(i_out);
             }
 
+            // nGraph's slice op does not like it when end < begin, so we truncate for that case
+            // here.
+            if (real_end < real_begin)
+            {
+                real_end = real_begin;
+            }
+
             // Compute output dimension.
             size_t dim = (real_end <= real_begin
                               ? 0

test/ref_generators/generate_dyn_slice_ref.py

@@ -15,6 +15,13 @@
 # limitations under the License.
 # ******************************************************************************
 
+#
+# Test case generator for DynSlice op.
+#
+# TODO(amprocte): try to get compile time of the generated code down, refactor
+# to use parameterized gtests.
+#
+
 import sys
 import numpy as np
 
@@ -202,6 +209,31 @@ def print_shape(dims):
 
     return 'Shape{' + ','.join(strs) + '}'
 
+#
+# Class to intercept indexing operations and write an nGraph C++ test case. The
+# generated test case will ensure that the output is identical to that which
+# would be produced by numpy on a "linspaced" array of the given shape and
+# dtype. If numpy throws an exception when the slice is attempted, the test
+# checks that nGraph throws some exception somewhere in graph construction or
+# execution, but does not attempt to make sure that the exception is the
+# "correct" one.
+#
+# Example usage:
+#
+#    w = SliceTestWriter(stream=sys.stdout)
+#
+#    # behave as if slicing a 4x5x6 input array of data type int32
+#    w.set_shape(4,5,6)
+#    w.set_dtype('int32')
+#
+#    # generate test cases for various behaviors, writing C++ code to sys.stdout
+#    w[0,:,:]
+#    w[...,-1:-3,:]
+#    w[-3,...,-1:-6:-3]
+#    w[1,2,3,4]        # note: in numpy this throws exception (too many
+#                      # indices), so generated test case will check that
+#                      # nGraph throws some exception too.
+#
 class SliceTestWriter:
     def __init__(self, shape=(), dtype='int32', stream=sys.stdout):
         self._shape = shape
@@ -214,12 +246,57 @@ class SliceTestWriter:
 
     def write_test(self, slices):
         data_in = np.linspace(0,np.prod(self._shape)-1,np.prod(self._shape),dtype=self._dtype).reshape(self._shape)
-        data_out = data_in.__getitem__(slices)
+
         n_slices = 1
         try:
             n_slices = len(slices)
         except TypeError:
             pass
+
+        try:
+            data_out = data_in.__getitem__(slices)
+        except Exception as e:
+            self._stream.write('    {\n')
+            self._stream.write('        auto arg = std::make_shared<op::Parameter>(%s, %s);\n' % (np_dt_to_ng(self._dtype), print_shape(self._shape)))
+            self._stream.write('        auto lb = std::make_shared<op::Parameter>(element::i64, %s);\n' % print_shape((n_slices,)))
+            self._stream.write('        auto ub = std::make_shared<op::Parameter>(element::i64, %s);\n' % print_shape((n_slices,)))
+            self._stream.write('        auto strides = std::make_shared<op::Parameter>(element::i64, %s);\n' % print_shape((n_slices,)))
+            self._stream.write('\n')
+            self._stream.write('        std::vector<%s> input_values(%s);\n' % (np_dt_to_c(self._dtype), np.prod(self._shape)))
+            self._stream.write('        std::iota(input_values.begin(), input_values.end(), static_cast<%s>(0));\n' % np_dt_to_c(self._dtype))
+            self._stream.write('        std::vector<int64_t> lb_values{%s};\n' % print_lb_values(slices))
+            self._stream.write('        std::vector<int64_t> ub_values{%s};\n' % print_ub_values(slices))
+            self._stream.write('        std::vector<int64_t> strides_values{%s};\n' % print_stride_values(slices))
+            self._stream.write('        AxisSet lb_mask{%s};\n' % print_lb_mask_axes(slices))
+            self._stream.write('        AxisSet ub_mask{%s};\n' % print_ub_mask_axes(slices))
+            self._stream.write('        AxisSet new_mask{%s};\n' % print_new_mask_axes(slices))
+            self._stream.write('        AxisSet shrink_mask{%s};\n' % print_shrink_mask_axes(slices))
+            self._stream.write('        AxisSet ellipsis_mask{%s};\n' % print_ellipsis_mask_axes(slices))
+            self._stream.write('\n')
+            self._stream.write('        // numpy threw: %s\n' % type(e))
+            self._stream.write('        EXPECT_ANY_THROW({\n');
+            self._stream.write('            auto slice = std::make_shared<op::DynSlice>(arg, lb, ub, strides, lb_mask, ub_mask, new_mask, shrink_mask, ellipsis_mask);\n')
+            self._stream.write('\n')
+            self._stream.write('            auto f = std::make_shared<Function>(NodeVector{slice}, ParameterVector{arg, lb, ub, strides});\n')
+            self._stream.write('\n')
+            self._stream.write('            auto backend = runtime::Backend::create("${BACKEND_NAME}",true);\n')
+            self._stream.write('            auto ex = backend->compile(f);\n')
+            self._stream.write('\n')
+            self._stream.write('            auto input_arg = backend->create_tensor(%s, %s);\n' % (np_dt_to_ng(self._dtype), print_shape(self._shape)))
+            self._stream.write('            auto input_lb = backend->create_tensor(element::i64, %s);\n' % print_shape((n_slices,)))
+            self._stream.write('            auto input_ub = backend->create_tensor(element::i64, %s);\n' % print_shape((n_slices,)))
+            self._stream.write('            auto input_strides = backend->create_tensor(element::i64, %s);\n' % print_shape((n_slices,)))
+            self._stream.write('            copy_data(input_arg, input_values);\n')
+            self._stream.write('            copy_data(input_lb, lb_values);\n')
+            self._stream.write('            copy_data(input_ub, ub_values);\n')
+            self._stream.write('            copy_data(input_strides, strides_values);\n')
+            self._stream.write('\n')
+            self._stream.write('            auto output = backend->create_dynamic_tensor(%s, PartialShape::dynamic());\n' % np_dt_to_ng(self._dtype))
+            self._stream.write('\n')
+            self._stream.write('            ex->call({output}, {input_arg, input_lb, input_ub, input_strides});\n')
+            self._stream.write('        });\n')
+            self._stream.write('    }\n')
+        else:
             self._stream.write('    {\n')
             self._stream.write('        auto arg = std::make_shared<op::Parameter>(%s, %s);\n' % (np_dt_to_ng(self._dtype), print_shape(self._shape)))
             self._stream.write('        auto lb = std::make_shared<op::Parameter>(element::i64, %s);\n' % print_shape((n_slices,)))
@@ -276,7 +353,8 @@ def main():
     assert(len(sys.argv) > 1)
 
     f = open(sys.argv[1], 'w')
-    f.write('''//*****************************************************************************
+    f.write('''\
+//*****************************************************************************
 // Copyright 2017-2019 Intel Corporation
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
@@ -330,14 +408,21 @@ NGRAPH_TEST(${BACKEND_NAME}, dyn_slice)
 
     t = SliceTestWriter(stream=f)
 
-    t.set_dtype('int32')
-
     t.set_shape((4,))
+    for dt in ['int32','float32']:
+        t.set_dtype(dt)
+
         t[np.newaxis,3:0:-1]
         t[...]
         t[1:3]
         t[2]
         t[3:0:-2]
+        t[3::-2]
+        t[4::-2]
+        t[5::-2]
+        t[-9000:-8000:2]
+        t[-9000:8000:2]
+        t[-5:5:2]
         t[np.newaxis]
         t[np.newaxis,np.newaxis]
         t[np.newaxis,np.newaxis,...,np.newaxis]
@@ -351,24 +436,39 @@ NGRAPH_TEST(${BACKEND_NAME}, dyn_slice)
         t[0:100:2]
         t[4:0:-2]
         t[4:0:-3]
-    # FIXME: DynElimination is not properly handling this (end < begin).
-    # t[3:2:1]
+        t[3:2:1]
         t[4::-2]
 
+    t.set_dtype('int32')
+    t[80000] # error expected (shrink-axis OOB)
+    t[-80000] # error expected (shrink-axis OOB)
+    t[:,:] # error expected (too many indices)
+    t[0:0:0] # error expected (stride==0)
+    t[0:1:0] # error expected (stride==0)
+    t[0:2:0] # error expected (stride==0)
+    t[::0] # error expected (stride==0)
+
     t.set_shape((2,3,4))
+    for dt in ['int32','float32']:
+        t.set_dtype(dt)
+
         t[1,np.newaxis]
         t[-1,-1,np.newaxis]
 
     t.set_shape((2,4,6,8,2,2,2))
+    for dt in ['int32','int64','float32','uint32']:
+        t.set_dtype(dt)
         t[0:,:4,2:6:2,7:3:-2,np.newaxis,...,1]
 
-    f.write('''
+    t.set_dtype('int32')
+    t[...,...] # error expected (too many ellipses)
+
+    f.write('''\
 }
 // clang-format on
 ''')
 
     f.close()
 
-
 if __name__ == "__main__":
     main()