[microNPU] Add support for SIGMOID by ekalda · Pull Request #9627 · apache/tvm

ekalda · 2021-12-01T16:40:03Z

Add support for SIGMOID activation function using the lookup
table mechanism in the NPU.

ekalda · 2021-12-01T16:43:20Z

cc @manupa-arm @lhutton1 @NicolaLancellotti @dchauhan-arm

lhutton1

LGTM Overall! Just a couple minor things I noticed :)

lhutton1 · 2021-12-02T13:56:32Z

python/tvm/relay/backend/contrib/ethosu/legalize.py

+    return y
+
+
+class SigmoidRewriter(DFPatternCallback):


This looks very similar to TanhRewriter, is it worth creating a subclass?

Good idea! I made the change

lhutton1 · 2021-12-02T13:58:52Z

python/tvm/relay/op/contrib/ethosu.py

+
+    def is_valid(self):
+        """
+        This function checks whether reshape has compatible attributes with the NPU


Suggested change

This function checks whether reshape has compatible attributes with the NPU

This function checks whether sigmoid has compatible attributes with the NPU

lhutton1 · 2021-12-02T14:01:53Z

tests/python/contrib/test_ethosu/test_codegen.py

+
+        class Model(tf.Module):
+            @tf.function
+            def tanh_function(self, x):


Suggested change

def tanh_function(self, x):

def sigmoid_function(self, x):

NicolaLancellotti · 2021-12-02T17:04:00Z

tests/python/contrib/test_ethosu/test_codegen.py

+    dtype = "int8"
+
+    def create_tflite_graph():
+        tf.config.run_functions_eagerly(True)


Suggested change

tf.config.run_functions_eagerly(True)

We don't need it, do we?

No we don't :D

ekalda · 2021-12-06T12:06:43Z

Thanks for the review @lhutton1!

NicolaLancellotti

Some type hints are missing, could you add them?

NicolaLancellotti · 2021-12-06T12:19:50Z

python/tvm/relay/backend/contrib/ethosu/legalize.py

+        self.activation_type = activation_type
+        self.calc_func = calc_func

    def callback(self, pre, post, node_map):


Suggested change

def callback(self, pre, post, node_map):

def callback(self, pre: tvm.relay.Expr, post: tvm.relay.Expr, node_map: tvm.ir.container.Map):

NicolaLancellotti · 2021-12-06T12:20:33Z

python/tvm/relay/backend/contrib/ethosu/legalize.py

        pass


+def sigmoid_calc_func(x):


Suggested change

def sigmoid_calc_func(x):

def sigmoid_calc_func(x: float) -> float:

NicolaLancellotti · 2021-12-06T12:21:28Z

python/tvm/relay/backend/contrib/ethosu/legalize.py

+    """A class to create an identity operator with the LUT"""

-    def __init__(self):
+    def __init__(self, params_class, activation_type, calc_func):


Suggested change

def __init__(self, params_class, activation_type, calc_func):

def __init__(self, params_class: Type, activation_type: string, calc_func: Callable[[float], float]):

NicolaLancellotti · 2021-12-06T12:24:46Z

python/tvm/relay/backend/contrib/ethosu/legalize.py


-def find_tanh_values(ifm_scale, ifm_zp, ofm_scale, ofm_zp):
-    """Method to calculate the values of the tanh lookup table"""
+def get_lut_from_func(ifm_scale, ifm_zp, ofm_scale, ofm_zp, func):


Suggested change

def get_lut_from_func(ifm_scale, ifm_zp, ofm_scale, ofm_zp, func):

def get_lut_from_func(ifm_scale: float, ifm_zp: int, ofm_scale: float, ofm_zp: int, func: Callable[[float], float]) -> list[int]:

lhutton1

Apologies, something I missed in my first review. Not essential, but given @NicolaLancellotti's review I suppose there's no harm in including this also :)

lhutton1 · 2021-12-06T12:41:10Z

python/tvm/relay/op/contrib/ethosu.py

    return pattern


+class SigmoidParams:


Should we also create a subclass for this and TanhParams?

Add support for SIGMOID activation function using the lookup table mechanism in the NPU.

ekalda · 2021-12-07T13:43:43Z

Thanks for the reviews @NicolaLancellotti and @lhutton1! :)

lhutton1

Thanks @ekalda, LGTM!

NicolaLancellotti

LGTM!

manupak

LGTM.

manupak · 2021-12-08T11:25:35Z

Thanks all!

Add support for SIGMOID activation function using the lookup table mechanism in the NPU.

ekalda requested review from MarisaKirisame, ZihengJiang, anijain2305, areusch, comaniac, icemelon, jroesch, junrushao, jwfromm, mbrookhart, merrymercy, slyubomirsky, tqchen, vinx13, wweic, yzhliu and zhiics as code owners December 1, 2021 16:40

ekalda mentioned this pull request Dec 1, 2021

[Tracking Issue] Arm(R) Ethos(TM)-U NPU operator support #9410

Closed

15 tasks

lhutton1 reviewed Dec 2, 2021

View reviewed changes

NicolaLancellotti reviewed Dec 2, 2021

View reviewed changes

ekalda force-pushed the sigmoid_upstream branch from a550c44 to 90ac839 Compare December 6, 2021 12:04

NicolaLancellotti reviewed Dec 6, 2021

View reviewed changes

lhutton1 reviewed Dec 6, 2021

View reviewed changes

ekalda added 3 commits December 7, 2021 13:27

[microNPU] Add support for SIGMOID

53d611a

Add support for SIGMOID activation function using the lookup table mechanism in the NPU.

Rebase and fix the review commits

534c35c

Respond to review comments

f818ef3

ekalda force-pushed the sigmoid_upstream branch from 90ac839 to f818ef3 Compare December 7, 2021 13:31

lhutton1 approved these changes Dec 7, 2021

View reviewed changes

NicolaLancellotti approved these changes Dec 7, 2021

View reviewed changes

manupak approved these changes Dec 8, 2021

View reviewed changes

manupak merged commit 3371a76 into apache:main Dec 8, 2021

ekalda deleted the sigmoid_upstream branch December 8, 2021 11:27

mikepapadim pushed a commit to mikepapadim/tvm that referenced this pull request Dec 9, 2021

[microNPU] Add support for SIGMOID (apache#9627)

94eda1a

Add support for SIGMOID activation function using the lookup table mechanism in the NPU.

ylc pushed a commit to ylc/tvm that referenced this pull request Jan 7, 2022

[microNPU] Add support for SIGMOID (apache#9627)

d2cfcba

Add support for SIGMOID activation function using the lookup table mechanism in the NPU.

yangulei pushed a commit to yangulei/tvm that referenced this pull request Jan 11, 2022

[microNPU] Add support for SIGMOID (apache#9627)

77c001d

Add support for SIGMOID activation function using the lookup table mechanism in the NPU.

yangulei pushed a commit to yangulei/tvm that referenced this pull request Jan 12, 2022

[microNPU] Add support for SIGMOID (apache#9627)

0a010a0

Add support for SIGMOID activation function using the lookup table mechanism in the NPU.

ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022

[microNPU] Add support for SIGMOID (apache#9627)

4c54686

Add support for SIGMOID activation function using the lookup table mechanism in the NPU.

qsqqsqqsq-intellif pushed a commit to qsqqsqqsq-intellif/tvm that referenced this pull request Apr 29, 2022

[microNPU] Add support for SIGMOID (apache#9627)

2bf64d3

Add support for SIGMOID activation function using the lookup table mechanism in the NPU.

driazati mentioned this pull request Jul 14, 2022

TVM v0.9.0.rc0 Release Candidate Notes #12102

Closed

	This function checks whether reshape has compatible attributes with the NPU
	This function checks whether sigmoid has compatible attributes with the NPU

	def callback(self, pre, post, node_map):
	def callback(self, pre: tvm.relay.Expr, post: tvm.relay.Expr, node_map: tvm.ir.container.Map):

	def sigmoid_calc_func(x):
	def sigmoid_calc_func(x: float) -> float:

	def __init__(self, params_class, activation_type, calc_func):
	def __init__(self, params_class: Type, activation_type: string, calc_func: Callable[[float], float]):

	def get_lut_from_func(ifm_scale, ifm_zp, ofm_scale, ofm_zp, func):
	def get_lut_from_func(ifm_scale: float, ifm_zp: int, ofm_scale: float, ofm_zp: int, func: Callable[[float], float]) -> list[int]:

Conversation

ekalda commented Dec 1, 2021

Uh oh!

ekalda commented Dec 1, 2021

Uh oh!

lhutton1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ekalda commented Dec 6, 2021

Uh oh!

NicolaLancellotti left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lhutton1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ekalda commented Dec 7, 2021

Uh oh!

lhutton1 left a comment

Choose a reason for hiding this comment

Uh oh!

NicolaLancellotti left a comment

Choose a reason for hiding this comment

Uh oh!

manupak left a comment

Choose a reason for hiding this comment

Uh oh!

manupak commented Dec 8, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments