Export testcase #105
Export testcase #105
Conversation
onnx_chainer/export_testcase.py
Outdated
|
|
||
|
|
||
| def makedirs(d): | ||
| if not os.path.exists(d): |
|
|
||
| chainer.config.train = True | ||
| model.cleargrads() | ||
| result = model(*args) |
There was a problem hiding this comment.
[optional] Wouldn't it be better to call export from this function?
I think current export_testcase does not output model.onnx file. I think almost all users who call export_testcase will need to call export as well. If a user calls both export and export_testcase, model(*args) will run twice, which is sub-optimal.
Maybe it's worth considering to add a function like export_impl which returns outputs in addition to model and this function uses it instead of running the model by this line.
onnx_chainer/export_testcase.py
Outdated
| out_dir (str): The directory name used for saving the input and output. | ||
| """ | ||
| makedirs(out_dir) | ||
| onnx_extra_inputs = [] |
There was a problem hiding this comment.
Line 28-30 are only for chainer-compiler and we shouldn't need them.
|
I changed the interface of the code so that the function saves the model file "model.onnx" as well. |
|
Yeah, I agree it's OK to run the model twice. I think we should eventually support at least list/tuple outputs, but this PR is a great start. I'd like to defer to tanaka-san. |
onnx_chainer/export_testcase.py
Outdated
| """ | ||
| os.makedirs(out_dir, exist_ok=True) | ||
| export(model, args, | ||
| filename='%s/model.onnx' % out_dir, |
There was a problem hiding this comment.
please use os.path.join function to make file path. L31 is same.
There was a problem hiding this comment.
Thank you for check. Fixed.
I agree, I also have my mind to the code assumed only one result, but I think dealing with multiple outputs will be supported when needed. |
I ported the implementation of test case export in
chainer_compilerhttps://github.com/pfnet-research/chainer-compiler/blob/master/scripts/onnx_chainer_util.py.Some functionalities such as outputting gradients and replacing ID are omitted.
Question?:
chainer.config.train=True? This might be incompatible with theexportfunction.