[Relay][FastMath] Relay pass to use fast exp/tanh #4873
Conversation
src/relay/pass/fast_math.cc
Outdated
|
|
||
| /*! | ||
| * \file fast_math.cc | ||
| * \brief Replaces non linear activation functions with their fast but appproximate counterparts. |
src/relay/pass/fast_math.cc
Outdated
| #include <tvm/relay/attrs/nn.h> | ||
| #include <tvm/relay/transform.h> | ||
| #include <tvm/relay/op.h> | ||
| #include "./pattern_util.h" |
|
Thanks @zhiics for quick review. I will incorporate your comments once the parent PR gets merged in. |
anijain2305
force-pushed
the
alex
branch
2 times, most recently
from
aa603e6
to
2ff78b7
Compare
|
I have rebased. I am not sure if opt_level=4 is the right way. There are other optimizations as well at opt_level=4, which I might not want to use. At the same time, it does not look like opt_level=3 candidate as well because it approximates. @zhiics |
There was a problem hiding this comment.
@anijain2305 In this case, I would think we can probably introduce a fast_math option to enable it and set the pass to the minimal level it requires to execute, although we are usually quite conservative in adding options to build config.
|
@tqchen any thoughts on this? |
|
I agree with @zhiics . |
|
@tqchen you mean to add a fast_math config option, right? I will add one |
|
While we cannot add a config option for every new chocies of optimizations, fastmath seems to be a quite common one that I think might be OK |
|
I agree. |
|
I investigated more. I am also little uncomfortable about adding another Relay build config option. It changes API of PassContext/BuildConfig object. Our goal is to make the pass user-friendly. We can use required_pass (current way) to do that. For a TVM user, there is no difference between the two options Current wayAlternativeIf we decide to add Since both are same from TVM user perspective, and current way does not change PassContext API, I feel more comfortable with current design. |
|
Adding it to The only concern I have is that I am not sure if we want to require passes from the higher level? Also, what's the relationship among the passes at opt_level=4? should we run it with other two passes when opt_level=4 is provided? This is not specific to this PR, but I think we probably want a better documentation for the choice of different opt levels. |
|
Seems |
|
@zhiics I agree we need more clarity over the usage of opt_level=4. Thinking more about this, I feel that opt_level=4 is more like a placeholder for all the passes, that will not be a part of default Relay build pipeline. I don't think we intend to use opt_level=4 to call these passes. Calling these passes should be either from outside or by using As far as this PR is concerned, I think we are ok. |
|
It is a bit pre-mature to introduce additional opt levels as we add passes. how about we only support the |
|
Yes, I am okay to use required_pass for now. |
| fast_mod = FastMath()(mod) | ||
| assert "fast_exp" in fast_mod.astext() | ||
|
|
||
| # Check that opt level 4 triggers the transformation. |
There was a problem hiding this comment.
Yes, thanks. I missed changing this one.
python/tvm/relay/transform.py
Outdated
| Returns | ||
| ------- | ||
| ret: tvm.relay.Pass | ||
| The registered to perform operator simplification. |
There was a problem hiding this comment.
Sorry, one more change. Please use force push.
The registered pass
* [Relay][FastMath] Relay pass to use fast exp/tanh * Adding required_pass to the tests. * FastMath test changes.
* [Relay][FastMath] Relay pass to use fast exp/tanh * Adding required_pass to the tests. * FastMath test changes.
As Title, dependent on the following PR - #4790
@alexgl-github @zhiics @masahi