One: Compiler FE: Support max_pool_with_argmax

Created on 8 Oct 2020  路  9Comments  路  Source: Samsung/ONE

Let's support max_pool_with_argmax Op in _luci_

Currently this is converted to CUSTOM(MaxPoolWithArgMax) in TF to TFlite.

DRAFT: #4646

  • [x] tflchef: #4719
  • [x] recipes: #4721

For #1928

Most helpful comment

According our current design decision everything is done, closing.
Thank you!

All 9 comments

@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!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

underflow101 picture underflow101  路  4Comments

seanshpark picture seanshpark  路  3Comments

binarman picture binarman  路  3Comments

KimDongEon picture KimDongEon  路  4Comments

lucenticus picture lucenticus  路  3Comments