[Relay][Pass] Only allow Module -> Module for opts managed by pass infra #3430

zhiics · 2019-06-25T07:53:07Z

This PR moves some passes from to the pass manager. The summary of the changes:

to a normal form implicitly modifies the module but only returns an updated expr. This may cause problems since some updated functions in the module might be lost. I changed to it a module pass that returns a module to avoid losing information.
Some unit tests in DCE don't have typing info. We can run Expr -> Expr opts on this them without types. However, as relay is strongly typed (at least everything in a module should be strongly typed), this PR adds types to them.
Partial Evaluation seems to need some more work for moving into the pass infra. I think it should be a module pass because it could evaluate the functions and then apply DCE to delete some of the dead ones. It looks to me we might need a global DCE pass to work on the module level and be able to delete functions. Otherwise, we might need to rely on inlining and outlining.

cc @jroesch @tqchen @MarisaKirisame @wweic @icemelon9 @slyubomirsky

jroesch · 2019-06-25T18:50:28Z

python/tvm/relay/transform.py

+    raise TypeError("Unknow mode: {}".format(mode))
+
+
+def OptimizeOnExpr(expr, passes):


shouldn't this be a function instead of a class style pass constructor?

Yes, I was using this just to keep it like an optimization as the other ones.

MarisaKirisame · 2019-06-28T04:11:58Z

for the ANF pass, the 'implicit update' is needed to traverse every global var used. however, with function module, this is not needed anymore. maybe you can remove the implicit update of the code?

for PE, I think it should be a Mod -> Mod at the impl level, and I will fix it.

MarisaKirisame · 2019-06-28T04:18:17Z

@zhiics can you not pass 'higher_order' rn? There isnt much case where the other mode is supported, and I personally think it should be deprecated, or at least, it should only be for expert who know what they are doing.
Also, what do you mean by AD dont support polymorphism? can you give me the error?

zhiics · 2019-06-28T04:48:32Z

@MarisaKirisame I tried to fix to ANF and PE in the commit. You can probably take a look.

For higher_order, I just kept it the same as what we had before. I can remove it later.
For the AD error, it is produced in the test_pow, you can probably check out and test it.

tests/python/relay/test_pass_dead_code_elimination.py

tests/python/relay/test_pass_partial_eval.py

MarisaKirisame · 2019-06-28T05:01:43Z

after some thinking, phasing AD as a pass is really bad.
AD do not preserve the semantic, and it do not preserve the type. It is not something u can just call - in this sense it is closer to freevars then to a pass.
Making it a pass just give people footgun to shoot themselves. (for example, ppl might pass a module into the AD pass, and transform all function to the AD mode).
but this is not how it is supposed to be use at all! It will probably just fail to type check, but for now, IMO it is best to make it just a standalone function, while I work on training and figure out how to make it part of our story.

can you revert this change? thank you.

zhiics · 2019-06-28T05:33:17Z

@MarisaKirisame Okay. I will revert the gradient pass. Later, we will have an analysis file and put the analysis passes there.

MarisaKirisame · 2019-06-28T05:45:48Z

@zhiics i agree. Thx.

zhiics · 2019-06-28T06:08:59Z

@MarisaKirisame reverted

tests/python/relay/test_pass_dead_code_elimination.py

tqchen · 2019-06-28T15:55:48Z

Re: AD as a Pass. I still think it should be Module->Module, we just need to debate about the APIs.
For example, the AD pass could take arguments of the name of interest("main") and differentiate that function, produce another function "main_diff" and related function if necessary.

MarisaKirisame · 2019-06-28T15:59:50Z

@tqchen this is a reasonable design, however there are other ways to do this, and I am thinking of doing a redesign/re-implementation of the AD. I feel like it could be delayed a bit.

Regardless, having it as FunctionPass seems like the wrong design.

tqchen · 2019-06-28T16:01:24Z

The main motivation is that we want to consolidate everything under Module->Module. If @MarisaKirisame you can quickly come up with a Module->Module style API proposal, I recommend we do so, given that it won't affect implementation as much.

zhiics · 2019-06-28T17:14:00Z

Yes, name of interest looks like a plausible way. I will keep this pass not touched in this PR.

jroesch · 2019-07-01T19:50:08Z

Re: AD we should add a special relay.grad operator, which attributes configure the AD pass. When we run the pass we will expand all static grad calls into their AD code.

…fra (apache#3430) * [Relay][Pass] Only allow Module -> Module for opts managed by pass infra * revert gradient pass

zhiics force-pushed the only_module branch 2 times, most recently from e9494d0 to 2624da0 Compare June 25, 2019 08:14

jroesch reviewed Jun 25, 2019

View reviewed changes

tqchen assigned jroesch Jun 27, 2019

zhiics mentioned this pull request Jun 27, 2019

[Relay][transform] Clean up pass.h #3312

Merged

5 tasks

zhiics force-pushed the only_module branch 2 times, most recently from 8621b9e to 3c4714a Compare June 28, 2019 04:44

zhiics force-pushed the only_module branch from 3c4714a to e165fd8 Compare June 28, 2019 04:51

MarisaKirisame requested changes Jun 28, 2019

View reviewed changes

tests/python/relay/test_pass_dead_code_elimination.py Outdated Show resolved Hide resolved

tests/python/relay/test_pass_partial_eval.py Outdated Show resolved Hide resolved

zhiics force-pushed the only_module branch from a48f1b6 to 28f424f Compare June 28, 2019 06:32

MarisaKirisame requested changes Jun 28, 2019

View reviewed changes

tests/python/relay/test_pass_dead_code_elimination.py Outdated Show resolved Hide resolved

zhiics added 2 commits June 28, 2019 16:53

[Relay][Pass] Only allow Module -> Module for opts managed by pass infra

d954284

revert gradient pass

01bb282

zhiics force-pushed the only_module branch from 28f424f to 01bb282 Compare June 28, 2019 17:09

MarisaKirisame approved these changes Jun 28, 2019

View reviewed changes

wweic approved these changes Jun 28, 2019

View reviewed changes

jroesch approved these changes Jul 1, 2019

View reviewed changes

jroesch merged commit f2a6851 into apache:master Jul 1, 2019

zhiics deleted the only_module branch July 2, 2019 21:49

wweic pushed a commit to wweic/tvm that referenced this pull request Jul 11, 2019

[Relay][Pass] Only allow Module -> Module for opts managed by pass in…

fe3f274

…fra (apache#3430) * [Relay][Pass] Only allow Module -> Module for opts managed by pass infra * revert gradient pass

wweic pushed a commit to wweic/tvm that referenced this pull request Jul 11, 2019

[Relay][Pass] Only allow Module -> Module for opts managed by pass in…

19fba9f

…fra (apache#3430) * [Relay][Pass] Only allow Module -> Module for opts managed by pass infra * revert gradient pass

wweic pushed a commit to neo-ai/tvm that referenced this pull request Jul 11, 2019

[Relay][Pass] Only allow Module -> Module for opts managed by pass in…

fe59b23

…fra (apache#3430) * [Relay][Pass] Only allow Module -> Module for opts managed by pass infra * revert gradient pass

tqchen mentioned this pull request Nov 8, 2019

[RELEASE][DRAFT] TVM v0.6 Release candidate #4259

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Relay][Pass] Only allow Module -> Module for opts managed by pass infra #3430

[Relay][Pass] Only allow Module -> Module for opts managed by pass infra #3430

zhiics commented Jun 25, 2019 •

edited

Loading

jroesch Jun 25, 2019

zhiics Jun 25, 2019

MarisaKirisame commented Jun 28, 2019

MarisaKirisame commented Jun 28, 2019

zhiics commented Jun 28, 2019

MarisaKirisame commented Jun 28, 2019 •

edited

Loading

zhiics commented Jun 28, 2019

MarisaKirisame commented Jun 28, 2019 •

edited

Loading

zhiics commented Jun 28, 2019

tqchen commented Jun 28, 2019

MarisaKirisame commented Jun 28, 2019 •

edited

Loading

tqchen commented Jun 28, 2019

zhiics commented Jun 28, 2019 •

edited

Loading

jroesch commented Jul 1, 2019

		raise TypeError("Unknow mode: {}".format(mode))


		def OptimizeOnExpr(expr, passes):

[Relay][Pass] Only allow Module -> Module for opts managed by pass infra #3430

[Relay][Pass] Only allow Module -> Module for opts managed by pass infra #3430

Conversation

zhiics commented Jun 25, 2019 • edited Loading

jroesch Jun 25, 2019

Choose a reason for hiding this comment

zhiics Jun 25, 2019

Choose a reason for hiding this comment

MarisaKirisame commented Jun 28, 2019

MarisaKirisame commented Jun 28, 2019

zhiics commented Jun 28, 2019

MarisaKirisame commented Jun 28, 2019 • edited Loading

zhiics commented Jun 28, 2019

MarisaKirisame commented Jun 28, 2019 • edited Loading

zhiics commented Jun 28, 2019

tqchen commented Jun 28, 2019

MarisaKirisame commented Jun 28, 2019 • edited Loading

tqchen commented Jun 28, 2019

zhiics commented Jun 28, 2019 • edited Loading

jroesch commented Jul 1, 2019

zhiics commented Jun 25, 2019 •

edited

Loading

MarisaKirisame commented Jun 28, 2019 •

edited

Loading

MarisaKirisame commented Jun 28, 2019 •

edited

Loading

MarisaKirisame commented Jun 28, 2019 •

edited

Loading

zhiics commented Jun 28, 2019 •

edited

Loading