-
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
[QNN] InferType changes that missed CI. #3779
Conversation
mod = relay.Module.from_expr(data[0]) | ||
mod = relay.transform.InferType()(mod) | ||
entry = mod["main"] | ||
data0 = entry if isinstance(data[0], relay.Function) else entry.body |
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.
Why not use from ...frontend.common import infer_type
. It seems unnecessary duplication of code. If the namespace is a problem we should probably move the existing method to a better place?
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.
Also, it seems the method from ...frontend.common import infer_type
has more checks.
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.
It seems strange to import pass like infer_type
from frontend
. Should we consider create shortcut for infer_type
? @jroesch
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.
Shortcut seems like a good idea.
Maybe, we can get this in to unblock CI and then follow up with another PR for shortcut and related changes to remove code duplication.
@vinx13 @ZihengJiang
Added a quick fix to unblock CI. Will think about requantize default output_dtype to remove the dependence on InferType.