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

[RFC] Add Float8E4M3FNUZ and Float8E5M2FNUZ to StableHLO. #1342

Merged
merged 4 commits into from
Apr 5, 2023

Conversation

jakeh-gc
Copy link
Contributor

This is a proposal to add Float8E4M3FNUZ and Float8E5M2FNUZ floating point types to StableHLO.
Feedback welcome, see rfcs/20230321-fp8_fnuz.md for more details.

@jakeh-gc jakeh-gc changed the title Propose adding Float8E4M3FNUZ and Float8E5M2FNUZ to StableHLO. [RFC] Propose adding Float8E4M3FNUZ and Float8E5M2FNUZ to StableHLO. Mar 21, 2023
@jakeh-gc jakeh-gc changed the title [RFC] Propose adding Float8E4M3FNUZ and Float8E5M2FNUZ to StableHLO. [RFC] Add Float8E4M3FNUZ and Float8E5M2FNUZ to StableHLO. Mar 21, 2023
@burmako burmako added the RFC label Mar 22, 2023
@burmako burmako self-assigned this Mar 22, 2023
@burmako
Copy link
Contributor

burmako commented Mar 24, 2023

@jakeh-gc Thank you for your RFC! Given its similarity to the F8E4M3FN and F8E5M2 RFC as well as the F8E4M3B11FNUZ RFC, I think that we have a strong case for the dtypes proposed in this RFC as well.

Similarly to the latter RFC which we have just approved, let's keep this one open for two weeks, i.e. until Wednesday 4/5, and then approve as well if no blocking feedback from the community.

@sherhut
Copy link

sherhut commented Mar 28, 2023

As stated in #1308 (comment), adding fp8 formats supported by hardware makes sense to me. The question about longer term remains. @burmako how would we organize work to explore address this? @jakeh-gc have you thought about precision_config?

Just to be clear: I am not asking for this longer-term design questions to block addition of the type itself.

Copy link

@cantonios cantonios left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from minor comment/question, LGTM.

rfcs/20230321-fp8_fnuz.md Outdated Show resolved Hide resolved
@jakeh-gc
Copy link
Contributor Author

@sherhut

@jakeh-gc have you thought about precision_config?

I'm not sure how precision_config should be dealt with. I'm not personally a fan of how it's defined at the moment:

* `DEFAULT`: Fastest calculation, but least accurate approximation to the
  original number.
* `HIGH`: Slower calculation, but more accurate approximation to the
  original number.
* `HIGHEST`: Slowest calculation, but most accurate approximation to the
  original number.

For an implementation, DEFAULT and HIGH seem like a license to do anything. I hope #755 resolves this 🤞.

My approach has been to at least use the precision of the input types, but that's certainly not endorsed by all of my colleagues who want "benchmarks" to go fast.

rfcs/20230321-fp8_fnuz.md Outdated Show resolved Hide resolved
@burmako
Copy link
Contributor

burmako commented Apr 5, 2023

Thank you, everyone, for the lively discussion! It's been two weeks, and we have broad support for adding the 2 new dtypes, with all the feedback addressed. The RFC is now approved.

A recurring theme in the discussions of the last few weeks has been working out a long-term story for the fp8 dtypes. I think this is important to follow up on, and I'll work with @theadactyl on including this on the agenda of our community meetings.

@burmako burmako merged commit 980db60 into openxla:main Apr 5, 2023
@burmako burmako mentioned this pull request Apr 13, 2023
GleasonK pushed a commit that referenced this pull request Apr 19, 2023
As proposed in [RFC: Float8E4M3FNUZ and
Float8E5M2FNUZ](https://github.com/openxla/stablehlo/blob/main/rfcs/20230321-fp8_fnuz.md)
(#1342), this change adds support for these types to StableHLO.

This includes the type definitions, vhlo, and interpreter support. The
testing approach mirrors the BFloat16 tests, since it is also a
"non-standard" floating point type supported by StableHLO.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

9 participants