-
Notifications
You must be signed in to change notification settings - Fork 81
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
[RFC] [Relay] Automatic Mixed Precision Pass #6
Conversation
Thanks for the RFC. I have two questions:
|
Yep that is correct it is very similar to the layout conversion pass. This RFC has an initial PR here: apache/tvm#8069. To answer your questions:
|
Thanks for the answers. I'll review the PR to get more implementation details. |
It should be trivial (hope I don't eat my words). I'm not 100% sure of the support for bfloat16 in current relay ops however. |
I don't know Chris Sullivan's github handle so if someone could cc him too that would be great. |
CCing @csullivan |
TVM has limited bfloat16 support now but it's on the way, so it would be better for this RFC to also consider this case, even the initial version may not cover it. |
So the associated PR is getting closer to a mergeable state. Is this RFC ready for more comments? |
cc @comaniac would be great if you can help shepherd this RFC |
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.
The concept and algorithm look good to me, but it would be better to provide more implementation/design details.
rfcs/0001-AMP_pass.md
Outdated
We can support automatic mixed precision retraining though that is a much, much larger future goal. It's | ||
good to have this in the meantime. |
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.
The answer to this question should come with a discussion of existing mechanisms used by other frameworks, such as XLA and PyTorch.
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.
Done. Please let me know if this is sufficient. Don't have the best background on some of this stuff.
Thanks for driving this review @comaniac. I'll get to this later in the week. |
Going to get to this tomorrow 😬. Promise 🤞 |
btw, according to #17, please update the RFC number on the file name to align with this PR number. |
Took a quick pass to the updated RFC. I think it's almost ready to merge as long as the last 3 comments are resolved. |
PTAL @comaniac |
@comaniac, I'll be talking about this at the TVM community meeting tomorrow so put off merging until after. |
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.
LGTM. Will merge after the community meeting if there's no objection.
If there is not other objections, this will be merged on monday. |
Thanks @AndrewZhaoLuo |
@AndrewZhaoLuo will it remove cast weight to float16 from graph? and make weight as float16 when build lib. |
@MeJerry215 Yes, casting of weight to fp16 is done at compile time by |
uma-rfc: update to questions/comments added
Relevant Links:
https://discuss.tvm.apache.org/t/rfc-relay-fp32-fp16-model-support/9994
apache/tvm#8069
cc @hogepodge @mbrookhart @anijain2305 @masahi
Link to tracking issue: apache/tvm#8296