u99127 commented on pull request #5528:
URL: https://github.com/apache/incubator-tvm/pull/5528#issuecomment-624774462
Thanks for the discussion and prompting this. I agree with you entirely with
the motivation behind this that we should look to simplify the frontend, the
amount of copy pasting to add a new operator isn't ideal and reducing the
complexity is a good thing.
>
>
> I thought of adding all the op attributes in table/map but decorator
seemed to be more pythonic.
Ok.
>
> Few more points to consider:
>
> 1. Sometimes equality checks can not be straight forward like
num_inputs==input_tensors. In this case we can always set num_input check as
None and handle assertion in the specific convert function.
>
That seems to be equivalent to -1 in the other scheme . So if we can't
handle it in the common case because it has variable number of inputs but
greater than a particular number for instance we need to make an operator
specific check.
I guess my question is whether a single decorator would be able to handle
the most common cases. At the end of the day my goal is to reduce code
duplication as much as I can avoid it which is why the table driven approach
helps but as you say it has it's limits especially with the cases you mention.
From a very old branch of mine, here is a table with the operator, helper
function, number of inputs , number of outputs : all of which are likely to
need just an equality check for number of inputs and outputs.
'AVERAGE_POOL_2D': (self.convert_average_pool2d, 1, 1),
'SOFTMAX': (self.convert_softmax, 1, 1),
'SQUEEZE': (self.convert_squeeze, 1, 1),
'MAX_POOL_2D': (self.convert_max_pool2d, 1, 1),
'ADD': (self.convert_add, 2, 1),
'SUB': (self.convert_sub, 2, 1),
'MUL': (self.convert_mul, 2, 1),
'DIV': (self.convert_div, 2, 1),
'POW': (self.convert_pow, 2, 1),
'MAXIMUM': (self.convert_maximum, 2, 1),
'MINIMUM': (self.convert_minimum, 2, 1),
'GREATER': (self.convert_greater, 2, 1),
'LOGISTIC': (self.convert_logistic, 1, 1),
'TANH':(self.convert_tanh, 1, 1),
'RELU':(self.convert_relu, 1, 1),
'CAST': (self.convert_cast, 1, 1),
'TRANSPOSE_CONV': (self.convert_transpose_conv, 3, 1)
as these have fixed number of input and output tensors.
My hunch is that we should be able to get to a single decorator for this
sample set above falls out but I'd like to see what you think. Without working
it out
If on the other hand we end up with multiple decorators per helper for each
of these , then we end up with duplication of code again and copy-pastism which
I think is what we both want to avoid.
> 2. Need to check instead of passing input tensors can we pass Relay
expressions. If input tensors are needed they can always be accessed using 'op'
variable.
>
What would the relay expressions represent ?
> 3. Need to check if there is a scope to add helper function for
quantized params calculation.
I'm not sure I follow this one.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
