Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for tflite arg_min and arg_max #5992

Merged
merged 6 commits into from
Jul 13, 2020

Conversation

d-smirnov
Copy link
Contributor

@d-smirnov d-smirnov commented Jul 3, 2020

"Add support for tflite arg_min and arg_max"

This is an updated version of abandoned PR [Relay][Frontend][TFLite] Add parser support for arg_min_max #4704 rebased on top of current master with small updates related to tflite 2.1.0.

Please note unit tests for arg_max and arg_min emit following warning:
/workspace/src/te/schedule/bound.cc:119: not in feed graph consumer = compute(T_multiply_red_temp, 0x53f5050)

inadob and others added 2 commits July 3, 2020 18:52
* this implementation supports only the case when the axis is a scalar
* tflite 1.13 removes all dims of size 1, Relay doesn't do this
* WARNING: every newer version of tflite > 1.13 needs keepdims=TRUE
keepdims set to False and added some checks

Note the unit tests emmitted following warning:
/workspace/src/te/schedule/bound.cc:119: not in feed graph consumer = compute(T_multiply_red_temp, 0x53f5050)
@d-smirnov d-smirnov changed the title Arg min max updated Add support for tflite arg_min and arg_max Jul 3, 2020
@MarisaKirisame
Copy link
Contributor

@d-smirnov can you get some other ppl to review as well?

python/tvm/relay/frontend/tflite.py Outdated Show resolved Hide resolved
def _convert_arg_min_max(self, relay_op, op):
"""Generic method to convert TFLite arg_min_max"""
try:
from tflite.Operator import Operator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this. not required

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link

@cam145 cam145 Jul 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@d-smirnov

is something like this?

def convert_arg_max(self, op):
    """Convert TFLite ARG_MAX"""
    if self.is_quantized(op):
        raise tvm.error.OpNotImplemented(
            'TFlite quantized ARG_MAX operator is not supported yet.')
    return self._convert_unary_elemwise(_op.arg_max, op)

python/tvm/relay/frontend/tflite.py Outdated Show resolved Hide resolved
python/tvm/relay/frontend/tflite.py Show resolved Hide resolved
python/tvm/relay/frontend/tflite.py Outdated Show resolved Hide resolved
Removed quantized argmin due to inablility to provide proper test case
Copy link
Member

@siju-samuel siju-samuel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@anijain2305 anijain2305 merged commit 712c82f into apache:master Jul 13, 2020
@anijain2305
Copy link
Contributor

Thanks @d-smirnov @siju-samuel @MarisaKirisame This is merged!

trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Jul 14, 2020
* [Relay][Frontend][TFLite] Add parser support for arg_min_max

* this implementation supports only the case when the axis is a scalar
* tflite 1.13 removes all dims of size 1, Relay doesn't do this
* WARNING: every newer version of tflite > 1.13 needs keepdims=TRUE

* Migrated to tflite 2.1.0

keepdims set to False and added some checks

Note the unit tests emmitted following warning:
/workspace/src/te/schedule/bound.cc:119: not in feed graph consumer = compute(T_multiply_red_temp, 0x53f5050)

* linter

* Removed quantized argmin

Removed quantized argmin due to inablility to provide proper test case

* added negative ranges

* re-trigger CI

Co-authored-by: Ina_Dobreva <Ina.Dobreva@arm.com>
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Jul 14, 2020
* [Relay][Frontend][TFLite] Add parser support for arg_min_max

* this implementation supports only the case when the axis is a scalar
* tflite 1.13 removes all dims of size 1, Relay doesn't do this
* WARNING: every newer version of tflite > 1.13 needs keepdims=TRUE

* Migrated to tflite 2.1.0

keepdims set to False and added some checks

Note the unit tests emmitted following warning:
/workspace/src/te/schedule/bound.cc:119: not in feed graph consumer = compute(T_multiply_red_temp, 0x53f5050)

* linter

* Removed quantized argmin

Removed quantized argmin due to inablility to provide proper test case

* added negative ranges

* re-trigger CI

Co-authored-by: Ina_Dobreva <Ina.Dobreva@arm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants