-
Notifications
You must be signed in to change notification settings - Fork 95
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
Add NANOO FP8 Support #2695
base: develop-upstream
Are you sure you want to change the base?
Add NANOO FP8 Support #2695
Conversation
7c84244
to
77f6214
Compare
6aa7078
to
8ecf02f
Compare
@@ -563,6 +563,14 @@ struct ProtoHelper<float8_e5m2> : public Float8ProtoHelper<float8_e5m2> {}; | |||
template <> | |||
struct ProtoHelper<float8_e4m3fn> : public Float8ProtoHelper<float8_e4m3fn> {}; | |||
|
|||
template <> |
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.
Nit: Format the code the same way as that for ocp fp8?
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 believe it's auto-formatted by clang-format
.
https://github.com/ROCm/tensorflow-upstream/blob/develop-upstream/tensorflow/.clang-format
@@ -271,8 +271,8 @@ def testBfloat16(self): | |||
def testFloat8e5m2(self): | |||
test_type = dtypes.float8_e5m2.as_numpy_dtype | |||
t = tensor_util.make_tensor_proto(np.array([10.0, 20.0], dtype=test_type)) | |||
# 10.0: "I" = 73 = 10010 01: 2^(18 - 15) * (1 + 1/4) | |||
# 20.0: "M" = 77 = 10011 01: 2^(19 - 15) * (1 + 1/4) | |||
# 10.0: "I" = 73 = 0 10010 01: 2^(18 - 15) * (1 + 1/4) |
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.
Nit: Why do we need the extra 0 here? For more clarity?
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.
Updated. (I just wanted to display all bit positions of FP8. But I agree that our repo should be consistent with the upstream repo as possible as we can, for less pain in the weekly sync.)
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.
Add NANOO FP8 Support To OPs
This PR adds
tf.experimental.float8_e4m3fnuz
andtf.experimental.float8_e5m2fnuz
as public tensorflow data types.similar work: