CNTK v2 library: Improve handling of certain error conditions

1) Better error handling in Axis normalization
2) Beter handling of bad convolution operand shapes
3) Switch NDShape::operator[] to checked indexing

Source/CNTKv2LibraryDll/API/CNTKLibrary.h

@@ -382,12 +382,18 @@ namespace CNTK
         ///
         /// Returns a reference to dimension size for the specified axis.
         ///
-        size_t& operator[](size_t axisId) { return m_shapeDims[axisId]; }
+        size_t& operator[](size_t axisId) 
+        {
+            return m_shapeDims.at(axisId); 
+        }
 
         ///
         /// Returns the dimension size for the specified axis.
         ///
-        size_t operator[](size_t axisId) const { return m_shapeDims[axisId]; }
+        size_t operator[](size_t axisId) const 
+        {
+            return m_shapeDims.at(axisId); 
+        }
 
         ///
         /// Creates and returns a new NDShape instance with the same dimensions as 'this' shape's specified axis range [beginAxisId, endAxisId).

Source/CNTKv2LibraryDll/PrimitiveFunction.cpp

@@ -640,7 +640,7 @@ namespace CNTK
                                 for (size_t i2 = 0; i2 < kernelRank; ++i2)
                                     m_inputs[0].m_dataFields->m_shape[i2] = kernelShape[i2];
                             }
-                            if (transpose && m_inputs[0].m_dataFields->m_shape[kernelRank] == NDShape::InferredDimension)
+                            if (transpose && (m_inputs[0].Shape().Rank() > kernelRank) && (m_inputs[0].Shape()[kernelRank] == NDShape::InferredDimension))
                                 m_inputs[0].m_dataFields->m_shape[kernelRank] = outputMapCount[outputMapCount.Rank()-1]; 
 
                             m_attributes[PrimitiveFunction::AttributeNameSharing] = AsDictionaryValueVector(sharing);
@@ -1015,8 +1015,8 @@ namespace CNTK
     }
 
     NDShape PrimitiveFunction::ConvolutionOpOutputShape(PrimitiveOpType op, const NDShape& operandShape, NDShape& kernelShape, NDShape& outputMapCount, NDShape& strides,
-        std::vector<bool>& sharing, std::vector<bool>& autoPad, NDShape& lowerPad, NDShape& upperPad,
-        bool transpose, bool inferDimensions, bool ceilOutputDim/* = false*/) const
+                                                        std::vector<bool>& sharing, std::vector<bool>& autoPad, NDShape& lowerPad, NDShape& upperPad,
+                                                        bool transpose, bool inferDimensions, bool ceilOutputDim/* = false*/) const
     {
         if (inferDimensions)
         {
@@ -1035,7 +1035,7 @@ namespace CNTK
             // picking the corresponding operand shape dimensionality
             // This is done by shrinking the filter rank and let the dimensions be inferred from the operand's shape
             // TODO: Should we do this for all of the axes in kernelShape that have a dimensionailty of NDShape::InferredDimension?
-            if (kernelShape[filterRank - 1] == NDShape::InferredDimension)
+            if ((filterRank > 0) && (kernelShape[filterRank - 1] == NDShape::InferredDimension))
             {
                 filterRank--;
                 kernelShape = kernelShape.SubShape(0, filterRank);

Source/CNTKv2LibraryDll/Utils.h

@@ -360,10 +360,13 @@ namespace CNTK
         NDShape kernelShape = convolutionMapShape.SubShape(0, operandShape.Rank());
         auto outputMapCount = convolutionMapShape.SubShape(kernelShape.Rank());
         auto shapeRank = operandShape.Rank(); 
-        NDShape paddedOutputMapCount(shapeRank, 1);
-        for (size_t i = 0; i < outputMapCount.Rank(); ++i)
-            paddedOutputMapCount[shapeRank - 1 - i] = outputMapCount[outputMapCount.Rank() - 1 - i];
-        if (transpose && paddedOutputMapCount[shapeRank - 1] == NDShape::InferredDimension)  // conovlution transpose, the mapCount in depth is derived from operandShape 
+        NDShape paddedOutputMapCount;
+        if (shapeRank > outputMapCount.Rank())
+            paddedOutputMapCount = NDShape(shapeRank - outputMapCount.Rank(), 1);
+
+        paddedOutputMapCount = paddedOutputMapCount.AppendShape(outputMapCount);
+
+        if (transpose && (shapeRank > 0) && (paddedOutputMapCount[shapeRank - 1] == NDShape::InferredDimension))  // convolution transpose, the mapCount in depth is derived from operandShape 
             paddedOutputMapCount[shapeRank - 1] = operandShape[shapeRank - 1];
 
         return{ paddedOutputMapCount, kernelShape };
@@ -506,7 +509,13 @@ namespace CNTK
             if (axis == Axis::EndStaticAxis())
                 axis = Axis((int)operandShape.Rank());
             else if (axis.StaticAxisIndex() < 0)
-                axis = Axis((int)operandShape.Rank() + axis.StaticAxisIndex());
+            {
+                auto normalizedAxis = Axis((int)operandShape.Rank() + axis.StaticAxisIndex());
+                if (normalizedAxis.StaticAxisIndex() < 0)
+                    InvalidArgument("Axis '%S' is out of bounds of the operand shape '%S' it applies to.", axis.AsString().c_str(), operandShape.AsString().c_str());
+                else
+                    axis = normalizedAxis;
+            }
         }
 
         return axis;

bindings/python/cntk/learners/tests/learner_test.py

@@ -10,6 +10,7 @@ from .. import *
 from cntk import parameter, input
 
 import pytest
+import sys
 
 LR_SCHEDULE_PARAMS = [
         ((0.2, UnitType.sample), [0.2]),
@@ -126,7 +127,7 @@ def test_learner_update():
 class TestProgressWriter(cntk_py.ProgressWriter):
 
     def __init__(self):
-        super(TestProgressWriter, self).__init__(1, 0, 1, 0)
+        super(TestProgressWriter, self).__init__(1, 0, 1, 0, sys.maxsize, 0)
         self.log_output = []
         self.__disown__()
 

bindings/python/cntk/ops/tests/function_tests.py

@@ -10,6 +10,7 @@ Unit tests for the function class.
 
 import numpy as np
 import pytest
+import cntk as C
 from ..functions import *
 from ...train.trainer import *
 from ...initializer import glorot_uniform
@@ -410,3 +411,13 @@ def test_update_signature():
 
     assert f.outputs[0].shape == (input_dim,)
     assert f.x.shape == (input_dim,)
+
+
+def test_transpose_0d_1d_operands():
+    x1 = C.input(())
+    with pytest.raises(ValueError):
+        transpose_0d = C.transpose(x1)
+
+    x2 = C.input(2)
+    with pytest.raises(ValueError):
+        transpose_1d = C.transpose(x2)

bindings/python/cntk/ops/tests/kernel_test.py

@@ -528,3 +528,17 @@ def test_convolution_transpose_with_output(input_size, conv_size, result, device
 
     unittest_helper(input_op, forward_input, expected_forward,
                     None, device_id=device_id, precision=precision)
+
+
+def test_conv_incorrect_shapes():
+    input = C.input(())    
+    with pytest.raises(ValueError):
+        h = C.layers.Convolution(filter_shape=(5,5), num_filters=8, strides=(1,1), pad=True)(input)
+    with pytest.raises(ValueError):
+        h = C.layers.MaxPooling(filter_shape=(2,2), strides=(2,2))(input)
+
+    input = C.input(28)    
+    with pytest.raises(ValueError):
+        h = C.layers.Convolution(filter_shape=(5,5), num_filters=8, strides=(1,1), pad=True)(input)
+    with pytest.raises(ValueError):
+        h = C.layers.MaxPooling(filter_shape=(2,2), strides=(2,2))(input)