Let's support max_pool_with_argmax Op in _luci_
Currently this is converted to CUSTOM(MaxPoolWithArgMax) in TF to TFlite.
DRAFT: #4646
For #1928
@lemmaa , About circle schema, we need to add our new one for this.
But the ID number may differ when TensorFlow adds this in TFlite schema and may result manual modifications.
How should we do with this Op?
@lucenticus , please take this item
Okay, sure! @d-krylov is going to start implementation of this operation.
@seanshpark, I heard opinions Luci IR has too much operators (and we shouldn't add them without good reason). This operator requires adding in Luci IR, as I can see. Can i do it?
I heard opinions Luci IR has too much operators (and we shouldn't add them without good reason).
Um? I wonder where that came from :)
This operator requires adding in Luci IR, as I can see. Can i do it?
Yes, please go ahead!
@seanshpark
Um? I wonder where that came from :)
If I remember it right this concern was related to circle schema, like there are limited space for op codes.
But for now there are a lot of space for new operators, so I am not sure what it actually was about.
@seanshpark
@glistening proposed to pass MaxPoolWithArgmax custom operator generated in tflite through Compiler as-is (https://github.com/Samsung/ONE/pull/4723#issuecomment-716238048 ) so we can avoid insertion of new opcode into circle.
I can not come up with any case when we need this operator as a separate entity so far, so maybe this proposal make sense.
Does this mean that we can consider this task completed if we merge tflchef, tests(receipts)?
What do you think?
P.s. I saw several "ResolveCustomOp" passes in luci, and thought we transform custom operators generated by tflite converter
into separate circle ops. But now I see that all these passes transform custom opcodes into existing operators.
so we can avoid insertion of new opcode into circle.
Technically, I also think going with CustomOp is better than adding MaxPoolWithArgmax and get conflict in the future.
I can not come up with any case when we need this operator as a separate entity so far,
Me too :)
Does this mean that we can consider this task completed if we merge tflchef, tests(receipts)? What do you think?
I think so. Had a short talk with @lemma and also agreed.
According our current design decision everything is done, closing.
Thank you!
Most helpful comment
According our current design decision everything is done, closing.
Thank you!