New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make type check errors more understandable #136

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@sinhrks
Contributor

sinhrks commented Jul 4, 2015

Recently added type-checking is very useful, but it looks not easy to read. This PR intends to make it more understandable by showing where the error is raised.

Assuming the following model (derived from MNIST example).

model = chainer.FunctionSet(l1=F.Linear(784, n_units),
                            l2=F.Linear(n_units + 5, n_units),  # dimensionality mismatch
                            l3=F.Linear(n_units, 10))

Current error

chainer.utils.type_check.InvalidType: Expect: in_types[0].shape[1] == W.shape[1]
Actual: 1000 != 1005

After this PR

chainer.utils.type_check.InvalidType: 
Invalid Operation is performed in: Linear (l2) (Forward)

Expect: in_types[0].shape[1] == W.shape[1]
Actual: 1000 != 1005

If base idea looks OK, I'll work on:

  • Add name property to all functions.
    • Add name attribute to Function and FunctionSet. FunctionSet will automatically attach name to given functions based on its key.
    • Add name to functions which has own __init__ method
  • Define Function and FunctionSet string representations
  • Tests
@unnonouno

This comment has been minimized.

Show comment
Hide comment
@unnonouno

unnonouno Jul 6, 2015

Member

That's great!!! Its base idea is OK.

We need discuss about a method to get a name of a function. I want to reserve __str__ and __repr__ to print debug message. So, how about make another method to get names of functions, such as .name ?

Member

unnonouno commented Jul 6, 2015

That's great!!! Its base idea is OK.

We need discuss about a method to get a name of a function. I want to reserve __str__ and __repr__ to print debug message. So, how about make another method to get names of functions, such as .name ?

@sinhrks sinhrks changed the title from (WIP) Make type check errors more understandable to Make type check errors more understandable Jul 11, 2015

@sinhrks

This comment has been minimized.

Show comment
Hide comment
@sinhrks

sinhrks Jul 11, 2015

Contributor

OK, once done. Adding some comments needs be discussed.

Contributor

sinhrks commented Jul 11, 2015

OK, once done. Adding some comments needs be discussed.

Show outdated Hide outdated chainer/function.py Outdated
Show outdated Hide outdated chainer/function.py Outdated
Show outdated Hide outdated chainer/function_set.py Outdated
Show outdated Hide outdated chainer/functions/basic_math.py Outdated
Show outdated Hide outdated chainer/functions/basic_math.py Outdated
@sinhrks

This comment has been minimized.

Show comment
Hide comment
@sinhrks

sinhrks Oct 7, 2015

Contributor

@unnonouno Sorry to leave a while. I've applied the same fix to functions added, leaving FunctionSet as it is (added validation only, not name). Can you take a look?

Contributor

sinhrks commented Oct 7, 2015

@unnonouno Sorry to leave a while. I've applied the same fix to functions added, leaving FunctionSet as it is (added validation only, not name). Can you take a look?

@sinhrks

This comment has been minimized.

Show comment
Hide comment
@sinhrks

sinhrks Oct 25, 2015

Contributor

Rebased and squashed. @unnonouno ?

Contributor

sinhrks commented Oct 25, 2015

Rebased and squashed. @unnonouno ?

@unnonouno

This comment has been minimized.

Show comment
Hide comment
@unnonouno

unnonouno Nov 4, 2015

Member

Thank you for fixing PR, and sorry for late reply.
We are now working on large refactoring at #573 including chainer.Function. So, please send a PR about changing error message of type-check first. And after #573, let's discuss about .name property.

Member

unnonouno commented Nov 4, 2015

Thank you for fixing PR, and sorry for late reply.
We are now working on large refactoring at #573 including chainer.Function. So, please send a PR about changing error message of type-check first. And after #573, let's discuss about .name property.

@sinhrks

This comment has been minimized.

Show comment
Hide comment
@sinhrks

sinhrks Nov 6, 2015

Contributor

Thanks for consideration. Issued #607 for error message improvement.

Contributor

sinhrks commented Nov 6, 2015

Thanks for consideration. Issued #607 for error message improvement.

@stale

This comment has been minimized.

Show comment
Hide comment
@stale

stale bot Oct 23, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 30 days if no further activity occurs. Thank you for your contributions.

stale bot commented Oct 23, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 30 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 23, 2017

@stale

This comment has been minimized.

Show comment
Hide comment
@stale

stale bot Nov 22, 2017

This issue is closed as announced. Feel free to re-open it if needed.

stale bot commented Nov 22, 2017

This issue is closed as announced. Feel free to re-open it if needed.

@stale stale bot closed this Nov 22, 2017

@mingxiaoh

This comment has been minimized.

Show comment
Hide comment
@mingxiaoh

mingxiaoh Dec 11, 2017

Contributor

recently when we were trying to do inference test for googlenet_v2, similar problem occurred. Would you please help have a look at this issue? @unnonouno
-----details-----
File "/home/mingxiao/pythonenv/py2-intel-chainer/lib/python2.7/site-packages/chainer/links/caffe/caffe_function.py", line 173, in call
output_vars = func(*input_vars)
File "/home/mingxiao/pythonenv/py2-intel-chainer/lib/python2.7/site-packages/chainer/links/caffe/caffe_function.py", line 572, in call
return self.caffe_func[self.name](*xs, **kwargs)
File "/home/mingxiao/pythonenv/py2-intel-chainer/lib/python2.7/site-packages/chainer/links/connection/linear.py", line 128, in call
return linear.linear(x, self.W, self.b)
File "/home/mingxiao/pythonenv/py2-intel-chainer/lib/python2.7/site-packages/chainer/functions/connection/linear.py", line 118, in linear
y, = LinearFunction().apply(args)
File "/home/mingxiao/pythonenv/py2-intel-chainer/lib/python2.7/site-packages/chainer/function_node.py", line 228, in apply
self._check_data_type_forward(in_data)
File "/home/mingxiao/pythonenv/py2-intel-chainer/lib/python2.7/site-packages/chainer/function_node.py", line 296, in _check_data_type_forward
self.check_type_forward(in_type)
File "/home/mingxiao/pythonenv/py2-intel-chainer/lib/python2.7/site-packages/chainer/functions/connection/linear.py", line 20, in check_type_forward
x_type.shape[1] == w_type.shape[1],
File "/home/mingxiao/pythonenv/py2-intel-chainer/lib/python2.7/site-packages/chainer/utils/type_check.py", line 519, in expect
expr.expect()
File "/home/mingxiao/pythonenv/py2-intel-chainer/lib/python2.7/site-packages/chainer/utils/type_check.py", line 477, in expect
'{0} {1} {2}'.format(left, self.inv, right))
chainer.utils.type_check.InvalidType:
Invalid operation is performed in: LinearFunction (Forward)

Expect: in_types[0].shape[1] == in_types[1].shape[1]
Actual: 128 != 512

Contributor

mingxiaoh commented Dec 11, 2017

recently when we were trying to do inference test for googlenet_v2, similar problem occurred. Would you please help have a look at this issue? @unnonouno
-----details-----
File "/home/mingxiao/pythonenv/py2-intel-chainer/lib/python2.7/site-packages/chainer/links/caffe/caffe_function.py", line 173, in call
output_vars = func(*input_vars)
File "/home/mingxiao/pythonenv/py2-intel-chainer/lib/python2.7/site-packages/chainer/links/caffe/caffe_function.py", line 572, in call
return self.caffe_func[self.name](*xs, **kwargs)
File "/home/mingxiao/pythonenv/py2-intel-chainer/lib/python2.7/site-packages/chainer/links/connection/linear.py", line 128, in call
return linear.linear(x, self.W, self.b)
File "/home/mingxiao/pythonenv/py2-intel-chainer/lib/python2.7/site-packages/chainer/functions/connection/linear.py", line 118, in linear
y, = LinearFunction().apply(args)
File "/home/mingxiao/pythonenv/py2-intel-chainer/lib/python2.7/site-packages/chainer/function_node.py", line 228, in apply
self._check_data_type_forward(in_data)
File "/home/mingxiao/pythonenv/py2-intel-chainer/lib/python2.7/site-packages/chainer/function_node.py", line 296, in _check_data_type_forward
self.check_type_forward(in_type)
File "/home/mingxiao/pythonenv/py2-intel-chainer/lib/python2.7/site-packages/chainer/functions/connection/linear.py", line 20, in check_type_forward
x_type.shape[1] == w_type.shape[1],
File "/home/mingxiao/pythonenv/py2-intel-chainer/lib/python2.7/site-packages/chainer/utils/type_check.py", line 519, in expect
expr.expect()
File "/home/mingxiao/pythonenv/py2-intel-chainer/lib/python2.7/site-packages/chainer/utils/type_check.py", line 477, in expect
'{0} {1} {2}'.format(left, self.inv, right))
chainer.utils.type_check.InvalidType:
Invalid operation is performed in: LinearFunction (Forward)

Expect: in_types[0].shape[1] == in_types[1].shape[1]
Actual: 128 != 512

@unnonouno

This comment has been minimized.

Show comment
Hide comment
@unnonouno

unnonouno Dec 11, 2017

Member

It suggests that "second dimension of first argument must be same as second dimension of second argument in LinearFunciton, but the former is 128 and later is 512". The first is x and the second is W in LInearFunction. I think x is input vector and its dimension is 128, but W is weight matrix in the model and its dimension is 512.

Member

unnonouno commented Dec 11, 2017

It suggests that "second dimension of first argument must be same as second dimension of second argument in LinearFunciton, but the former is 128 and later is 512". The first is x and the second is W in LInearFunction. I think x is input vector and its dimension is 128, but W is weight matrix in the model and its dimension is 512.

@mingxiaoh

This comment has been minimized.

Show comment
Hide comment
@mingxiaoh

mingxiaoh Dec 12, 2017

Contributor

@unnonouno As we dig the code deeper, we found that IntelChainer do reshape when the dimension missmatch, that is why IntelChainer works.

Contributor

mingxiaoh commented Dec 12, 2017

@unnonouno As we dig the code deeper, we found that IntelChainer do reshape when the dimension missmatch, that is why IntelChainer works.

@kmaehashi kmaehashi added this to the Closed issues and PRs milestone Dec 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment