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

[Frontend][Tensorflow] SelectV2 and BroadcastArgs op support for tf2 models #7901

Merged
merged 5 commits into from
Apr 24, 2021

Conversation

srinidhigoud
Copy link
Contributor

@srinidhigoud srinidhigoud commented Apr 21, 2021

Adding support in tf parser for new ops SelectV2 and BroadcastArgs introduced by tf2. This is focused on compiling and running the tf2 version of the object detection models such as faster rcnn model. Since both the inputs to BroadcastArgs were constant for every instance of the op in the model there was no need to write a relay op.

@srinidhigoud
Copy link
Contributor Author

srinidhigoud commented Apr 21, 2021

out.appendleft(s1[s1_size - i])
if s1_size < s0_size:
for i in range(s1_size + 1, s0_size + 1):
out.appendleft(s0[s0_size - i])
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it better to use itertools.zip_longest() on the reversed array instead of running three separate loops? Might be helpful to have fill_values set at 1 to avoid handling None values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will require atleast two list inversions (both the inputs as we have to iterate in reverse to verify broadcasting rules). Fill values will only make sense if I use a zip. It is only two loops (one of last two will fail) and merging them to one will give marginal optimization.

Copy link
Contributor

@comaniac comaniac left a comment

Choose a reason for hiding this comment

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

Please revert the change of updating submodules.
Also cc @yongwww @kevinthesun @zhiics

python/tvm/relay/frontend/tensorflow.py Outdated Show resolved Hide resolved
python/tvm/relay/frontend/tensorflow.py Show resolved Hide resolved
Copy link
Contributor

@comaniac comaniac left a comment

Choose a reason for hiding this comment

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

LGTM

@comaniac comaniac merged commit fad10d7 into apache:main Apr 24, 2021
@comaniac
Copy link
Contributor

Thanks @srinidhigoud @rohanmukh

echuraev pushed a commit to echuraev/tvm that referenced this pull request Apr 29, 2021
umangyadav pushed a commit to umangyadav/tvm that referenced this pull request May 5, 2021
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request May 6, 2021
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request May 6, 2021
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request May 6, 2021
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants