search

chainer / onnx-chainer

Add-on package for ONNX format support in Chainer
MIT License
85 stars 24 forks source link

Stop error when ksize/pad/stride condition is not satisfied #209

Closed ir5 closed 5 years ago

ir5 commented 5 years ago

Current converters of pooling operators don't allow ksize/pad/stride that does not satisfy some constraint. This constraint is quite specific to the specification of ONNX-runtime, and so the export of the layer should be allowed even when the constraint is not satisfied. I changed raise RuntimeError part to warnings.warn(...).

codecov-io commented 5 years ago

Codecov Report

Merging #209 into master will increase coverage by 0.23%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #209      +/-   ##
==========================================
+ Coverage   89.69%   89.93%   +0.23%     
==========================================
  Files          24       24              
  Lines        1417     1430      +13     
==========================================
+ Hits         1271     1286      +15     
+ Misses        146      144       -2
Impacted Files Coverage Δ
onnx_chainer/functions/pooling.py 88.73% <ø> (+5.59%) :arrow_up:
onnx_chainer/functions/math.py 85.71% <0%> (-0.54%) :arrow_down:
onnx_chainer/functions/__init__.py 100% <0%> (ø) :arrow_up:
onnx_chainer/mapping.py 90% <0%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 88994fb...6eb5957. Read the comment docs.

disktnk commented 5 years ago

I agree to ease constraints about cover_all option.

On this comment, the exception block is for output value test with ONNXRuntime. Actually, MXNet does not stop calculation with such a condition (but output value is not match with Chainer).

How about those?

  1. remove exception and test for it
  2. add comment about ONNXRuntime constrains "Pad should be smaller than kernel"
ir5 commented 5 years ago

Thank you for comment. Now I removed warnings and just left comments there.