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

[WINDOWS] Fix python exception Creation in Windows #4529

Closed
kice opened this issue Dec 16, 2019 · 10 comments
Closed

[WINDOWS] Fix python exception Creation in Windows #4529

kice opened this issue Dec 16, 2019 · 10 comments

Comments

@kice
Copy link
Contributor

kice commented Dec 16, 2019

https://github.com/apache/incubator-tvm/blob/8e3b5d39c388f6767a2e69afff27eac55557567e/python/tvm/autotvm/task/task.py#L192

Looks like the C++ code dose not raise AttributeError, it just print the message.

https://github.com/apache/incubator-tvm/blob/8e3b5d39c388f6767a2e69afff27eac55557567e/src/node/reflection.cc#L108-L111

Should we print the message or raise a error?

ref: #2279

@tqchen
Copy link
Member

tqchen commented Dec 16, 2019

see #2838 it will automatically translate to the corresponding error in the python side. For further development related discussions, please open a new thread on https://discuss.tvm.ai/:)

@tqchen tqchen closed this as completed Dec 16, 2019
@kice
Copy link
Contributor Author

kice commented Dec 17, 2019

In fact, on windows, it wont.

@tqchen
Copy link
Member

tqchen commented Dec 17, 2019

hmm, interesting finding. This might be a bug that we can fix, could relates to \n \r difference

@tqchen tqchen reopened this Dec 17, 2019
@tqchen tqchen changed the title Raise a python exception or Print a message [WINDOWS] Fix python exception Creation in Windows Dec 17, 2019
@tqchen
Copy link
Member

tqchen commented Dec 17, 2019

reopen and label as help wanted, @kice can you dig a bit further?

@kice
Copy link
Contributor Author

kice commented Dec 17, 2019

Here return C error string

https://github.com/apache/incubator-tvm/blob/10392854076a369bd8dfdf4d4044cad917285345/src/node/reflection.cc#L109-L110

[23:21:56] C:\src\tvm-win\src\node\reflection.cc:109: AttributeError: Schedule object has no attributed code_hash

https://github.com/apache/incubator-tvm/blob/10392854076a369bd8dfdf4d4044cad917285345/python/tvm/_ffi/base.py#L192-L198

_find_error_type return None since after the split, err_name is [23:, thus _valid_error_name return False, which make get_last_ffi_error think it was not a py error and throw <class 'tvm._ffi.base.TVMError'>.

https://github.com/apache/incubator-tvm/blob/10392854076a369bd8dfdf4d4044cad917285345/python/tvm/_ffi/base.py#L175-L177

I would suggest split the c error string into tokens by :, and check every token to see if it is a py error.

I think we should add a api to query which kind of error was happened, instead of checking the error string.

But base on my analysis, this should also happen on Linux. Wired.

@tqchen
Copy link
Member

tqchen commented Dec 17, 2019

Here is the code that normalizes the error message https://github.com/apache/incubator-tvm/blob/master/src/runtime/c_runtime_api.cc#L203

which will removes the header, but somehow fails your case

@kice
Copy link
Contributor Author

kice commented Dec 18, 2019

https://ideone.com/LbSfYC

Looks like the : in the path cause the issue

@tqchen
Copy link
Member

tqchen commented Dec 18, 2019

@kice would you like to propose a fix ?

@kice
Copy link
Contributor Author

kice commented Dec 18, 2019

I would suggect to change the path on windows to unix like. So change the code in dmlc logging module? Add a constexpr to do the style conversion.

@tqchen
Copy link
Member

tqchen commented Dec 18, 2019

I think the main fix could be in the NormalizeError function to fix the windows case if possible

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants