Skip to content

[Torch] Clean up usage of try ... infer_value() ... except - #6504

Merged
masahi merged 5 commits into
apache:masterfrom
masahi:torch-infer-value-cleanup
Sep 22, 2020
Merged

[Torch] Clean up usage of try ... infer_value() ... except#6504
masahi merged 5 commits into
apache:masterfrom
masahi:torch-infer-value-cleanup

Conversation

@masahi

@masahimasahi commented Sep 17, 2020

Copy link
Copy Markdown
Member

This is a follow up to #6449 where some usage of exception-throwing infer_value is introduced. Following the discussion there, I attempted to clean up that usage with a new wrapper API. I think this is a reasonable change to add.

The following API is added. If this looks good, I'll add a doc string. See the example use in the Torch frontend.

def try_infer_value(val, on_success, on_failure):
try:
ret = infer_value(val, {}).asnumpy()
return on_success(ret), True
except Exception:
return on_failure(), False

please review @kevinthesun@t-vi@siju-samuel

@masahimasahi changed the title [Torch] Clean up usage of try ... infer value() ... except[Torch] Clean up usage of try ... infer_value() ... exceptSep 17, 2020
Comment threadpython/tvm/relay/frontend/pytorch.py
@masahi

masahi commented Sep 17, 2020

Copy link
Copy Markdown
MemberAuthor

@tqchen I got a build error from utvm crt https://ci.tvm.ai/blue/organizations/jenkins/tvm/detail/PR-6504/2/pipeline

I don't understand why I'm getting an error from a file like crcccitt.c that doesn't exist. Maybe a build cache problem?

@masahi
masahiforce-pushed the torch-infer-value-cleanup branch from 94f4627 to e6094feCompareSeptember 17, 2020 10:11
@leandron

Copy link
Copy Markdown
Contributor

@tqchen I got a build error from utvm crt https://ci.tvm.ai/blue/organizations/jenkins/tvm/detail/PR-6504/2/pipeline

I don't understand why I'm getting an error from a file like crcccitt.c that doesn't exist. Maybe a build cache problem?

Yeah, I'm seeing the same on my PR as well.

@tqchen

tqchen commented Sep 17, 2020

Copy link
Copy Markdown
Member

@masahi was due to a cmake cache problem that PR has been merged, please retrigger.

Comment threadpython/tvm/relay/frontend/common.py Outdated
@masahi
masahiforce-pushed the torch-infer-value-cleanup branch from e6094fe to 9ac55caCompareSeptember 21, 2020 20:40
Comment threadpython/tvm/relay/frontend/common.py Outdated

@kevinthesunkevinthesun left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@masahi
masahi merged commit 0448858 into apache:masterSep 22, 2020
@masahi

Copy link
Copy Markdown
MemberAuthor

Thanks @kevinthesun@yongwww

TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Oct 13, 2020
* clean up infer value usage
* try silence pylint
* remove unused variable
* make on_failuare optional
* make on_success optional True
Co-authored-by: masa <masa@pop-os.localdomain>
TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Oct 14, 2020
* clean up infer value usage
* try silence pylint
* remove unused variable
* make on_failuare optional
* make on_success optional True
Co-authored-by: masa <masa@pop-os.localdomain>
TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Oct 15, 2020
* clean up infer value usage
* try silence pylint
* remove unused variable
* make on_failuare optional
* make on_success optional True
Co-authored-by: masa <masa@pop-os.localdomain>
TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Oct 16, 2020
* clean up infer value usage
* try silence pylint
* remove unused variable
* make on_failuare optional
* make on_success optional True
Co-authored-by: masa <masa@pop-os.localdomain>
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Oct 19, 2020
* clean up infer value usage
* try silence pylint
* remove unused variable
* make on_failuare optional
* make on_success optional True
Co-authored-by: masa <masa@pop-os.localdomain>
Sign up for freeto 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.

5 participants

@masahi@leandron@tqchen@yongwww@kevinthesun