Support temporary implicit input #202

disktnk · 2019-07-25T09:52:01Z

fixes #201

def forward(self, x):
    x + chainer.Variable(...)

Then, second value's reference is lost when convert to graph, so converter cannot set the value into ONNX graph .

To keep such a value reference, exporter retains input values in forwarding using LinkHook and patched apply. By introducing LinkHook, the hook function only retains temporary values to reduce memory usage, but the target model is required to use forward function, not __call__ function.

temporary implicit inputs are set as "value"(initializer), not as "constant".
drop export_implicit_inputs_public option, because converter cannot distinguish the temporary value is covered by converter or not. For example, beta of batch-normalization is sometimes None but followed by converter.

shinh · 2019-07-25T09:56:06Z

onnx_chainer/export.py

@@ -103,11 +103,28 @@ def rename_variable_name(
        context.set_name(variables, new_name, pinned=True)


+class InputCacheHook(chainer.FunctionHook):


How about RetainInputHook? "cache" sounds like we have the real copy of the inputs

shinh · 2019-07-25T09:56:21Z

onnx_chainer/export.py

@@ -103,11 +103,28 @@ def rename_variable_name(
        context.set_name(variables, new_name, pinned=True)


+class InputCacheHook(chainer.FunctionHook):
+    """cache raw inputs


Suggested change

"""cache raw inputs

"""Retain raw inputs

shinh · 2019-07-25T10:16:09Z

onnx_chainer/export.py

+
+    def forward_postprocess(self, fn, in_data):
+        # in_data is ndarray, not variable
+        self.func_inputs[id(fn)] = in_data


I think this will increase memory consumption. I think what we need to keep here is in_data which is not wrapped by chainer.Variable in inputs of FunctionNode.forward. However, it seems to be impossible to check this?

Exactly! I tried LinkHook instead.

codecov-io · 2019-07-29T08:03:19Z

Codecov Report

Merging #202 into master will increase coverage by 0.18%.
The diff coverage is 97.05%.

@@            Coverage Diff             @@
##           master     #202      +/-   ##
==========================================
+ Coverage   89.69%   89.88%   +0.18%     
==========================================
  Files          24       24              
  Lines        1417     1473      +56     
==========================================
+ Hits         1271     1324      +53     
- Misses        146      149       +3

Impacted Files	Coverage Δ
onnx_chainer/graph.py	`95.65% <100%> (ø)`	⬆️
onnx_chainer/export.py	`91.47% <97.01%> (+1.47%)`	⬆️
onnx_chainer/functions/normalization.py	`86.44% <0%> (-6.78%)`	⬇️
onnx_chainer/functions/math.py	`85.71% <0%> (-0.54%)`	⬇️
onnx_chainer/functions/__init__.py	`100% <0%> (ø)`	⬆️
onnx_chainer/mapping.py	`90% <0%> (ø)`	⬆️
onnx_chainer/functions/pooling.py	`88.73% <0%> (+5.59%)`	⬆️

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...8148f6e. Read the comment docs.

disktnk · 2019-07-30T00:38:59Z

/test

pfn-ci-bot · 2019-07-30T00:39:02Z

Successfully created a job for commit 0a2d8ab:

Dashboard for commit 0a2d8ab

shinh · 2019-07-30T05:28:56Z

Sorry if I'm confused, but this does not work for the following case?

    def test_implicit_input_value(self):
        class Model(chainer.Chain):

            def __init__(self):
                super(Model, self).__init__()
                self.frac = np.array(4, dtype=np.float32)

            def forward(self, x):
                return x / self.frac

        x = chainer.Variable(np.array(1, dtype=np.float32))
        self.expect(Model(), x, name='implicit_input_value')

…r into fix/implicit-weakref

disktnk · 2019-07-30T07:06:05Z

Good catch. If inputs are ndarray type, function node converts them to chainer.Variable within itself, and will be lost out of function node (apply). I fixed it.

shinh

LGTM!!

shinh · 2019-07-31T03:50:48Z

onnx_chainer/export.py

+                if referenced_var is None:
+                    # This variable is created within function node and weakref
+                    # is lost. Make temporary variable and retain it.
+                    if isinstance(inputs[i], chainer.Variable):


temp_var = chainer.as_variable(inputs[i]) ?

disktnk · 2019-07-31T09:09:24Z

/test

pfn-ci-bot · 2019-07-31T09:09:26Z

Successfully created a job for commit 8148f6e:

Dashboard for commit 8148f6e

disktnk added 5 commits July 25, 2019 17:10

add failure test of implicit input

e4ac9cf

fix to add implicit input as const

81d6e7f

fix const and public test

acc5187

fix to support public input

574c744

fix test to be failed

e7622ee

shinh approved these changes Jul 25, 2019

View reviewed changes

disktnk added 4 commits July 26, 2019 18:31

add testcase

d539e8b

fix to change using link_hook

c859248

drop additinal input option

432049e

apply flake8

b10dbd9

disktnk changed the title ~~[WIP] Support implicit input as constant~~ Support temporary implicit input Jul 29, 2019

disktnk marked this pull request as ready for review July 29, 2019 08:15

disktnk added 2 commits July 29, 2019 17:23

fix to revert

cfce7eb

apply autopep8

0a2d8ab

disktnk force-pushed the fix/implicit-weakref branch from f8f6014 to 0a2d8ab Compare July 29, 2019 10:55

disktnk added 2 commits July 30, 2019 15:59

fix to support ndarray

72992fc

Merge branch 'fix/implicit-weakref' of github.com:disktnk/onnx-chaine…

8f8cbd4

…r into fix/implicit-weakref

fix replace test case

8bac6a1

shinh approved these changes Jul 31, 2019

View reviewed changes

disktnk added this to the 1.5.0 milestone Jul 31, 2019

fix not to use isinstance, fix comments

8148f6e

disktnk merged commit b1a1abe into chainer:master Jul 31, 2019

disktnk deleted the fix/implicit-weakref branch July 31, 2019 10:10

disktnk mentioned this pull request Aug 4, 2019

Validation Error "Nodes in a graph must be topologically sorted", tryed to export stylegan generator #214

Closed

disktnk mentioned this pull request Aug 17, 2019

Set unused variable on initializer, using fixed-batchnormalization #225

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Support temporary implicit input #202

Support temporary implicit input #202

Uh oh!

disktnk commented Jul 25, 2019 •

edited

Loading

Uh oh!

shinh Jul 25, 2019

Uh oh!

shinh Jul 25, 2019

Uh oh!

shinh Jul 25, 2019

Uh oh!

disktnk Jul 29, 2019

Uh oh!

codecov-io commented Jul 29, 2019 •

edited

Loading

Uh oh!

disktnk commented Jul 30, 2019

Uh oh!

pfn-ci-bot commented Jul 30, 2019

Uh oh!

shinh commented Jul 30, 2019

Uh oh!

disktnk commented Jul 30, 2019

Uh oh!

shinh left a comment

Uh oh!

shinh Jul 31, 2019

Uh oh!

disktnk commented Jul 31, 2019

Uh oh!

pfn-ci-bot commented Jul 31, 2019

Uh oh!

Uh oh!

		@@ -103,11 +103,28 @@ def rename_variable_name(
		context.set_name(variables, new_name, pinned=True)


		class InputCacheHook(chainer.FunctionHook):

Support temporary implicit input #202

Support temporary implicit input #202

Uh oh!

Conversation

disktnk commented Jul 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shinh Jul 25, 2019

Choose a reason for hiding this comment

Uh oh!

shinh Jul 25, 2019

Choose a reason for hiding this comment

Uh oh!

shinh Jul 25, 2019

Choose a reason for hiding this comment

Uh oh!

disktnk Jul 29, 2019

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Jul 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

disktnk commented Jul 30, 2019

Uh oh!

pfn-ci-bot commented Jul 30, 2019

Uh oh!

shinh commented Jul 30, 2019

Uh oh!

disktnk commented Jul 30, 2019

Uh oh!

shinh left a comment

Choose a reason for hiding this comment

Uh oh!

shinh Jul 31, 2019

Choose a reason for hiding this comment

Uh oh!

disktnk commented Jul 31, 2019

Uh oh!

pfn-ci-bot commented Jul 31, 2019

Uh oh!

Uh oh!

disktnk commented Jul 25, 2019 •

edited

Loading

codecov-io commented Jul 29, 2019 •

edited

Loading