Skip to content

[TVMC] compile: Check if FILE exists #10608

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.

Sign up for GitHub

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

Jump to bottom
Merged
masahi merged 1 commit into from
Mar 25, 2022
Merged

[TVMC] compile: Check if FILE exists #10608

masahi merged 1 commit into from
Mar 25, 2022

Conversation

@gromero
Copy link
Contributor

@gromero gromero commented Mar 14, 2022 •
edited
Loading

Currently when a non-existing FILE is passed to 'tvmc compile' it throws
a traceback because a FileNotFoundError exception is not handled. Since
there is no need for such abrupt exit, and the trace can also confuse
users, this commit fixes it by checking if FILE indeed exists, informing
the user about the non-existing FILE before exiting.

@gromero
Copy link
Contributor Author

gromero commented Mar 14, 2022

cc @leandron

leandron
leandron approved these changes Mar 15, 2022
View reviewed changes
Copy link
Contributor

@leandron leandron left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix @gromero!

@gromero
Copy link
Contributor Author

gromero commented Mar 15, 2022 •
edited
Loading

@leandron Thanks for the review.

Do you know what's not pleasing the CI now? The result says:


[2022-03-14T23:29:44.074Z] = 1681 passed, 1567 skipped, 14 xfailed, 1 xpassed, 115 warnings in 668.33s (0:11:08) =

I really have no clue what happens here...

@leandron
Copy link
Contributor

leandron commented Mar 15, 2022

Looks like a transient: see test_solution_consistency in the full log at https://ci.tlcpack.ai/blue/rest/organizations/jenkins/pipelines/tvm/branches/PR-10608/runs/1/nodes/314/steps/535/log/?start=0

@gromero
Copy link
Contributor Author

gromero commented Mar 15, 2022 •
edited
Loading

test_solution_consistency

@leandron hmm I see it now. But also it says:

[2022-03-14T23:07:09.942Z] This test is intentionally non-deterministic, if it fails please report it in github issue together with this seed 2499843125996260036
[2022-03-14T23:07:09.942Z] 
[2022-03-14T23:07:09.942Z] FAILED

and since it's in the CI, shouldn't we mark it as "xfail" ? I think reporting every time in HG it fails in the CI is a big ask no?

@gromero
Copy link
Contributor Author

gromero commented Mar 15, 2022

Anyways, I'm re-triggering 👍

@gromero gromero force-pushed the improve_error_handling_tvmc_compile branch 2 times, most recently from eb20470 to 7765793 Compare March 18, 2022 23:18
areusch
areusch reviewed Mar 23, 2022
View reviewed changes
python/tvm/driver/tvmc/compiler.py Outdated
try:
tvmc_model = frontends.load_model(args.FILE, args.model_format, args.input_shapes)
except FileNotFoundError:
raise TVMCException(f"Input file {args.FILE} does not exist!")
Copy link
Contributor

@areusch areusch Mar 23, 2022

Choose a reason for hiding this comment

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

what about just checking if os.exists(args.FILE) rather than assuming all FileNotFoundError relate to the input file?

Copy link
Contributor Author

@gromero gromero Mar 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@areusch yeah I was unsure about that option or the one I've used. Sure, let's use os.path. I just used os.path.isfile() instead of os.path.exists() because exists() will also return true if FILE is a directory, which is also invalid in that context. PTAL. Thanks!

@gromero
[TVMC] compile: Check if FILE exists
6dd1bc1 
Currently when a non-existing FILE is passed to 'tvmc compile' it throws
a traceback because a FileNotFoundError exception is not handled. Since
there is no need for such abrupt exit, and the trace can also confuse
users, this commit fixes it by checking if FILE indeed exists, informing
the user about the non-existing FILE before exiting.

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
@gromero gromero force-pushed the improve_error_handling_tvmc_compile branch from 7765793 to 6dd1bc1 Compare March 23, 2022 21:55
@gromero gromero changed the title [TVMC] compile: Gently handle incorrect input file [TVMC] compile: Check if FILE exists Mar 23, 2022
@masahi masahi merged commit b56f9e5 into apache:main Mar 25, 2022
gromero added a commit to gromero/tvm that referenced this pull request Apr 1, 2022
@gromero
[TVMC] tune: Check if FILE exists (apache#10608)
ee71d34 
Currently when a non-existing FILE is passed to 'tvmc tune' it throws
a traceback because a FileNotFoundError exception is not handled. Since
there is no need for such abrupt exit, and the trace can also confuse
users, this commit fixes it by checking if FILE indeed exists, kindly
informing the user about the non-existing FILE before exiting.

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
gromero added a commit to gromero/tvm that referenced this pull request Apr 4, 2022
@gromero
[TVMC] tune: Check if FILE exists (apache#10608)
0d2e3ab 
Currently when a non-existing FILE is passed to 'tvmc tune' it throws
a traceback because a FileNotFoundError exception is not handled. Since
there is no need for such abrupt exit, and the trace can also confuse
users, this commit fixes it by checking if FILE indeed exists, kindly
informing the user about the non-existing FILE before exiting.

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
gromero added a commit to gromero/tvm that referenced this pull request Apr 5, 2022
@gromero
[TVMC] tune: Check if FILE exists (apache#10608)
dffafa0 
Currently when a non-existing FILE is passed to 'tvmc tune' it throws
a traceback because a FileNotFoundError exception is not handled. Since
there is no need for such abrupt exit, and the trace can also confuse
users, this commit fixes it by checking if FILE indeed exists, kindly
informing the user about the non-existing FILE before exiting.
pfk-beta pushed a commit to pfk-beta/tvm that referenced this pull request Apr 11, 2022
@gromero
[TVMC] compile: Check if FILE exists (apache#10608)
9c249fd 
Currently when a non-existing FILE is passed to 'tvmc compile' it throws
a traceback because a FileNotFoundError exception is not handled. Since
there is no need for such abrupt exit, and the trace can also confuse
users, this commit fixes it by checking if FILE indeed exists, informing
the user about the non-existing FILE before exiting.

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
gromero added a commit to gromero/tvm that referenced this pull request Apr 14, 2022
@gromero
[TVMC] tune: Check if FILE exists (apache#10608)
9dab6b8 
Currently when a non-existing FILE is passed to 'tvmc tune' it throws
a traceback because a FileNotFoundError exception is not handled. Since
there is no need for such abrupt exit, and the trace can also confuse
users, this commit fixes it by checking if FILE indeed exists, kindly
informing the user about the non-existing FILE before exiting.
gromero added a commit to gromero/tvm that referenced this pull request Apr 20, 2022
@gromero
[TVMC] tune: Check if FILE exists (apache#10608)
978a97f 
Currently when a non-existing FILE is passed to 'tvmc tune' it throws
a traceback because a FileNotFoundError exception is not handled. Since
there is no need for such abrupt exit, and the trace can also confuse
users, this commit fixes it by checking if FILE indeed exists, kindly
informing the user about the non-existing FILE before exiting.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@areusch areusch areusch left review comments

@leandron leandron leandron approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gromero @leandron @areusch @masahi