Make type check utilities #95

Merged
merged 40 commits into from Jul 3, 2015

Projects

None yet

3 participants

@unnonouno
Contributor
  • Make an expression tree to validate types and show errors
  • Add a function to check types to Function. The function is called with expression trees
  • Make examples of type check

fixes #1

For example, when 4th dimension of 1st argument of an input is expected to be 2, you can write like this:

def ceck_type_forward(self, in_types):
    in_types[0].shape[3].should_be(2)

And, actually it is 1, a user can get a error message like this:

Expect: in_types[0].shape[3] == 2
Actual: 1 != 2
@delta2323 delta2323 commented on an outdated diff Jun 29, 2015
chainer/utils/type_check.py
+
+def get_types(data, is_grad):
+ assert(isinstance(data, tuple))
+
+ if is_grad:
+ name = 'out_types'
+ else:
+ name = 'in_types'
+
+ info = TypeInfoTuple(
+ get_type(name, i, x, is_grad) for i, x in enumerate(data))
+ info._set_name(name)
+ return info
+
+
+def get_type(name, index, array, accept_none):
@delta2323
delta2323 Jun 29, 2015 Member

If get_type is not called only from get_types, I think get_type should be in get_types to narrow its scope.

@delta2323 delta2323 commented on an outdated diff Jun 29, 2015
chainer/utils/type_check.py
+ name = '{0}[{1}]'.format(name, index)
+ self.name = name
+ self.shape = tuple(Shape(x, i, name) for i, x in enumerate(shape))
+ self.dtype = DtypeExpr(dtype, name)
+ self.ndim = Member(len(self.shape), name, 'ndim')
+
+ def is_none(self):
+ return self.dtype is None
+
+ def __str__(self):
+ return self.name
+
+
+class TypeInfoTuple(tuple):
+
+ def _set_name(self, name):
@delta2323
delta2323 Jun 29, 2015 Member

We can simply write type_info_tuple_instance.name = name instead of making a setter method.

@delta2323 delta2323 commented on an outdated diff Jun 29, 2015
chainer/utils/type_check.py
+
+
+class TypeInfoTuple(tuple):
+
+ def _set_name(self, name):
+ self.name = name
+
+ def size(self):
+ return Member(len(self), self.name, 'size')
+
+
+def get_types(data, is_grad):
+ assert(isinstance(data, tuple))
+
+ if is_grad:
+ name = 'out_types'
@delta2323
delta2323 Jun 29, 2015 Member

get_types is not restricted to the case where data is out_type, but is not a gradient. So, I think we should not use is_grad as a flag for specifying out_type.

@delta2323 delta2323 commented on an outdated diff Jun 29, 2015
chainer/utils/type_check.py
+ return UnaryOperator(_wrap(x), exp, priority, func)
+ return f
+
+
+def _make_bin_operator(exp, priority, func):
+ def f(x, y):
+ return BinaryOperator(_wrap(x), _wrap(y),
+ exp, priority, func)
+ return f
+
+
+def _flip(f):
+ return lambda x, y: f(y, x)
+
+
+_add = _make_bin_operator('+', 4, int.__add__)
@delta2323
delta2323 Jun 29, 2015 Member

Why these binary operators are outside of IntExpr and exposed? If it is not needed, I think it is better that they are in IntExpr class

unnonouno added some commits Jun 30, 2015
@unnonouno unnonouno Rename get_type to _get_type 237ba1d
@unnonouno unnonouno Remove _set_name
4d34ae1
@unnonouno unnonouno Remove redundant variables
ba1daf5
@unnonouno unnonouno changed the title from [WIP] Make type check utilities to Make type check utilities Jun 30, 2015
unnonouno added some commits Jul 1, 2015
@unnonouno unnonouno Make bool and int expr 360eb46
@unnonouno unnonouno Make expect function 5b49f57
@unnonouno unnonouno Use new style of type check
069b7c1
@unnonouno unnonouno Insert a missing space
8f227b4
@unnonouno
Contributor

In offline discussion, we decided to use this style:

def ceck_type_forward(self, in_types):
    type_check.expect(
        in_types[0].shape[3] == 2,
        in_types[0].shape[2] == 1,
    )
unnonouno added some commits Jul 1, 2015
@unnonouno unnonouno Make type check for concat
5a9eacc
@unnonouno unnonouno Change interface of get_types
9b2bbda
@unnonouno unnonouno Write document of type check functions
5b84d9b
@beam2d beam2d added the feature label Jul 2, 2015
@beam2d beam2d added this to the v1.1.0 milestone Jul 2, 2015
@delta2323 delta2323 self-assigned this Jul 2, 2015
@delta2323 delta2323 commented on the diff Jul 2, 2015
chainer/function.py
+
+ def check_type_forward(self, in_types):
+ """Checks types of input data before forward propagation.
+
+ Before :meth:`forward` is called, this function is called.
+ You need to validate types of input data in this function
+ using type-check utilities.
+
+ Args:
+ in_types: :class:`TypeInfoTuple` object which contains type
+ information of input data for :meth:`forward`.
+ """
+ pass
+
+ def check_type_backward(self, in_types, grad_types):
+ """Checks types of gradient data before back propagation.
@delta2323
delta2323 Jul 2, 2015 Member

Does this function check types of input data as well? In other words, if check_type_forward is passed. isn't it necessary to write a check which involves in_types only?

@delta2323
delta2323 Jul 2, 2015 Member

We need comments somewhere that backward should be called after forward is called

@unnonouno
unnonouno Jul 2, 2015 Contributor

I added a comment to check_type_backward

@delta2323 delta2323 commented on the diff Jul 2, 2015
chainer/functions/concat.py
@@ -22,6 +23,38 @@ class Concat(function.Function):
def __init__(self, axis=1):
self.axis = axis
+ def check_type_forward(self, in_types):
+ type_check.expect(in_types.size() > 0)
+ type_check.expect(in_types[0].ndim >
+ type_check.IntVariable(self.axis, 'axis'))
@delta2323
delta2323 Jul 2, 2015 Member

Can I write in_types[0].ndim.eval > self.axis instead? I think it is better because value of the left hand side is used afterwards as ndim.

@delta2323
delta2323 Jul 2, 2015 Member

By offline discussion, I found that it is necessary to wrap self.axis in order to print variable name axis when this expectation failed.

@delta2323 delta2323 commented on the diff Jul 2, 2015
chainer/functions/concat.py
+ type_check.expect(in_types.size() > 0)
+ type_check.expect(in_types[0].ndim >
+ type_check.IntVariable(self.axis, 'axis'))
+
+ ndim = in_types[0].ndim.eval()
+ for i in range(1, in_types.size().eval()):
+ type_check.expect(
+ in_types[0].dtype == in_types[i].dtype,
+ in_types[0].ndim == in_types[i].ndim,
+ )
+ for d in range(0, ndim):
+ if d == self.axis:
+ continue
+ type_check.expect(in_types[0].shape[d] == in_types[i].shape[d])
+
+ def check_type_backward(self, in_types, out_types):
@delta2323
delta2323 Jul 2, 2015 Member

Do we need dtype check between in_types and out_types?

@delta2323 delta2323 commented on the diff Jul 2, 2015
chainer/functions/softmax_cross_entropy.py
+ def check_type_forward(self, in_types):
+ type_check.expect(in_types.size() == 2)
+ x_type, t_type = in_types
+
+ type_check.expect(
+ x_type.dtype == numpy.float32,
+ x_type.ndim == 2,
+ t_type.dtype == numpy.int32,
+ t_type.ndim == 1,
+
+ x_type.shape[0] == t_type.shape[0],
+ )
+
+ def check_type_backward(self, in_types, out_types):
+ type_check.expect(
+ in_types.size() == 2,
@delta2323
delta2323 Jul 2, 2015 Member

This is already checked by check_type_forward

@delta2323 delta2323 commented on an outdated diff Jul 2, 2015
chainer/utils/type_check.py
+ def __str__(self):
+ return self.name
+
+
+class TypeInfoTuple(tuple):
+
+ def size(self):
+ return Member(len(self), self.name, 'size')
+
+
+def get_types(data, name, accept_none):
+ assert(isinstance(data, tuple))
+
+ info = TypeInfoTuple(
+ _get_type(name, i, x, accept_none) for i, x in enumerate(data))
+ # I don't know a method to set an atribute in an initializer of tuple.
@delta2323
delta2323 Jul 2, 2015 Member

typo: attribute

@delta2323 delta2323 commented on an outdated diff Jul 2, 2015
chainer/utils/type_check.py
+ def f(x, y):
+ return BoolBinaryOperator(x, y, exp, inv, func)
+ return f
+
+
+def _flip(f):
+ return lambda x, y: f(y, x)
+
+
+class Expr(object):
+
+ def __init__(self, priority):
+ self.priority = priority
+
+ def eval(self):
+ raise NotImplemented()
@delta2323
delta2323 Jul 2, 2015 Member

NotImplemented is not callable. Use NotImplementedError

@delta2323 delta2323 commented on an outdated diff Jul 2, 2015
chainer/utils/type_check.py
+ return f
+
+
+def _flip(f):
+ return lambda x, y: f(y, x)
+
+
+class Expr(object):
+
+ def __init__(self, priority):
+ self.priority = priority
+
+ def eval(self):
+ raise NotImplemented()
+
+ def __nonzero__(self):
@delta2323
delta2323 Jul 2, 2015 Member

I would like to add a comment why __nonzero__ is prohibited.

@delta2323 delta2323 commented on an outdated diff Jul 2, 2015
chainer/utils/type_check.py
+ if isinstance(self.rhs, Expr) and self.priority >= self.rhs.priority:
+ right = '(' + right + ')'
+
+ return '{0} {2} {1}'.format(left, right, self.exp)
+
+
+class IntBinaryOperator(BinaryOperator, Int):
+
+ def __init__(self, priority, lhs, rhs, exp, func):
+ BinaryOperator.__init__(self, priority, lhs, rhs, exp, func)
+
+
+class Bool(object):
+
+ def expect(self):
+ raise NotImplemented()
@delta2323
delta2323 Jul 2, 2015 Member

Use NotImplementedError

@delta2323 delta2323 commented on an outdated diff Jul 2, 2015
chainer/utils/type_check.py
+
+class BinaryOperator(Expr):
+
+ def __init__(self, priority, lhs, rhs, exp, func):
+ super(BinaryOperator, self).__init__(priority)
+ self.lhs = lhs
+ self.rhs = rhs
+ self.exp = exp
+ self.func = func
+
+ def eval(self):
+ left = self.eval_left()
+ right = self.eval_right()
+ return self.func(left, right)
+
+ def eval_left(self):
@delta2323
delta2323 Jul 2, 2015 Member

eval_left is used only inside BinaryOperator class

@delta2323 delta2323 commented on an outdated diff Jul 2, 2015
chainer/utils/type_check.py
+ self.rhs = rhs
+ self.exp = exp
+ self.func = func
+
+ def eval(self):
+ left = self.eval_left()
+ right = self.eval_right()
+ return self.func(left, right)
+
+ def eval_left(self):
+ if isinstance(self.lhs, Expr):
+ return self.lhs.eval()
+ else:
+ return self.lhs
+
+ def eval_right(self):
@delta2323
delta2323 Jul 2, 2015 Member

eval_right is used only inside BinaryOperator class

@delta2323 delta2323 commented on an outdated diff Jul 2, 2015
chainer/functions/accuracy.py
class Accuracy(function.Function):
+
+ def check_type_forward(self, in_types):
+ type_check.expect(in_types.size() == 2)
+ x_type, t_type = in_types
+
+ type_check.expect(
+ x_type.dtype == numpy.float32,
+ x_type.ndim == 2,
@delta2323
delta2323 Jul 2, 2015 Member

ndim of x_type need not to be 2. It can have trailing 1's which starts from third dimension (like (10, 3, 1, 1, 1 ...)).

Kenta OONO Remove assertion about type check
244a3cc
@delta2323 delta2323 commented on an outdated diff Jul 2, 2015
chainer/utils/type_check.py
+ # As infix operators are left-associative, we need to append parens
+ # when rhs has the same priority
+ # e.g. x << (y << z) != x << y << z
+ if isinstance(self.rhs, Expr) and self.priority >= self.rhs.priority:
+ right = '(' + right + ')'
+
+ return '{0} {2} {1}'.format(left, right, self.exp)
+
+
+class IntBinaryOperator(BinaryOperator, Int):
+
+ def __init__(self, priority, lhs, rhs, exp, func):
+ BinaryOperator.__init__(self, priority, lhs, rhs, exp, func)
+
+
+class Bool(object):
@delta2323
delta2323 Jul 2, 2015 Member

Bool is misunderstanding with bool (True or False)

unnonouno added some commits Jul 1, 2015
@unnonouno unnonouno Write a document 0306a3a
@unnonouno unnonouno Support right-assiciative operator and fix pow 087440e
@unnonouno unnonouno Fix concat test 1148683
@unnonouno unnonouno Fix typo 7e35d67
@unnonouno unnonouno Raise NotImplementedError 8120ec8
@unnonouno unnonouno Write a comment about __nonzero__ of Expr cfe009c
@unnonouno unnonouno Rename eval_left and eval_right b027ba7
@unnonouno unnonouno Fix type check of accuracy 678497b
@unnonouno unnonouno Rename Bool to Testable
4a64248
@unnonouno unnonouno Fix comment about check_type_backward
b60c1cd
@unnonouno unnonouno Remove unused variable
7412755
@delta2323

typo: independent of

@delta2323

input data -> input/gradient data

@delta2323

typo: tuple

@delta2323

typo: abstract

@delta2323

are -> be

@delta2323

First character of "Raise" should be lower-cased, or first character of when in the comments above should be upper-cased.

@delta2323

typo: function (remove "s")

@delta2323

typo: backward

@delta2323

I think 'When "some" expression is evaluated...' is appropriate because previous sentence does not indicate that the expression is in given expressions.

@delta2323

I would like you to add two things

  • Both of x==y and z==w call __nonzero__, which by default return True unless they are None.
  • The reason why and returns evaluation result of second term is that both terms return True.
@unnonouno unnonouno Fix typo
9f1a808
@delta2323
Member

LGTM 👍

@delta2323 delta2323 merged commit ab8f07a into master Jul 3, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@delta2323 delta2323 deleted the type-check branch Jul 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment