-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Improve NNVM to Relay conversion #2734
Conversation
@@ -441,6 +443,23 @@ def check_function(symbol, forward=None, backward=None, grad_input_vars=None, | |||
debug_stage = "running" | |||
nnvm_res = main_function(**np_inputs) | |||
|
|||
try: | |||
logging.debug("checking to_relay conversion") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this print every time this code is executed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it depends on the logging configuration, but, with our CI, this message will be printed only when an error occurs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, seems ok
Overall looks good, I could swear their were more tests written by me, but I think were moved/removed during someone's overhaul of the tests. |
* Improve NNVM to Relay conversion * fix pylint * support __lshift_scalar__, abs, ceil, floor, and trunc to pass CI
* Improve NNVM to Relay conversion * fix pylint * support __lshift_scalar__, abs, ceil, floor, and trunc to pass CI
* Improve NNVM to Relay conversion * fix pylint * support __lshift_scalar__, abs, ceil, floor, and trunc to pass CI
This PR adds to_relay() checks to our NNVM tests. This PR also includes some cleanups of to_relay, fixes of detected bugs, and supports of some operators to pass the test.
@jroesch @yzhliu @grwlf please help review the PR.