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

Introduce IEEE P3109 dtypes #122

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Introduce IEEE P3109 dtypes #122

wants to merge 4 commits into from

Conversation

awf
Copy link

@awf awf commented Nov 10, 2023

This PR represents the interim report of IEEE working group P3109, defining 8-bit float datatypes. The working group has representatives from across the industry, including existing FP8 hardware vendors, and floating point experts behind the IEEE-754 standard.

Features of these datatypes (see the interim report for detailed rationale)

  • Parameterized by precision p as in IEEE-754 (p includes the implicit leading mantisa bit so exponent width w is w = 8-p)
  • Range determined by maximum exponent emax = 2^(w-1)-1, following IEEE-754
  • Types are named float8_p3109_p{p} in Python, templated float8_p3109_p<p> in C++
  • Positive and negative infinity
  • No negative zero
  • Single NaN (in the 0x80 position)

See Colab:ML Dtypes.ipynb to experiment with this branch.

@awf
Copy link
Author

awf commented Nov 10, 2023

@hawkinsp @jakevdp For your consideration: the IEEE Working Group thought it would be useful to provide a public implementation of the proposed IEEE FP8 types, even though there is, naturally, no current hardware implementation of these types.

This will allow practitioners to experiment with a standardised range of types, parameterized over precision, and further inform future hardware and software decision making regarding FP formats.

@hawkinsp
Copy link
Collaborator

A couple of quick questions:

  • have the committee's recommendations stabilized? Are these the final states of these types and the final names?
  • do these alias any of the existing types?

@awf
Copy link
Author

awf commented Nov 10, 2023

  • have the committee's recommendations stabilized? Are these the final states of these types and the final names?

These recommendations represent the discussions so far of the committee. The major decisions:

  • Presence of infinities
  • Presence of negative zero
  • Number of NaNs
  • Definition of the "precision" and "emax" parameters

Have been formally voted on, and current work is on defining operations on these types.
However, it could happen that during the work on defining operations, we could discover some reason why these decisions need to be altered or reversed, and then the committee could vote to undo a previous vote. Indeed that should always be the case: until the job is complete, we can't be certain that we haven't missed something; but at the same time, we don't know now of any reasons this might need to change.

Regarding final names, once a standard is written, I would expect the float8_p3109 naming to change to something more permanent, e.g. float8_ieee, but we didn't want to presume that in this release.

  • do these alias any of the existing types?

As it happens, no -- none of the current types match the desiderata. There's a table at the back of the interim report summarizing the current status:

image

@awf
Copy link
Author

awf commented Nov 10, 2023

Regarding final names, once a standard is written, I would expect the float8_p3109 naming to change to something more permanent, e.g. float8_ieee, but we didn't want to presume that in this release.

I have tested global find and replace of _p3109_ with _ieee_ and all tests continue to pass, so that option should be fairly OK if/when it happens. Yes, it could be an annoying change on a large codebase, akin to switching to a new code formatter, but I still think it's preferable to assuming these are the final types.

Copy link
Collaborator

@jakevdp jakevdp left a comment

Choose a reason for hiding this comment

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

Thanks so much for this contribution! It all looks really great – a few minor comments below.

@hawkinsp and I were discussing whether we might merge these new types in some sort of experimental/provisional way, to make clear that they might change. Maybe something like a ml_dtypes.provisional or ml_dtypes.experimental namespace? What do you think?


@staticmethod
def _float8_p3109_p5_finfo():
return finfo._float8_p3109_p_finfo(5)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems strange for a static method to refer to its class by name...

Maybe we could remove these three and use cls._float8_p3109_p_finfo(p) directly in __new__ below

Copy link
Author

@awf awf Nov 11, 2023

Choose a reason for hiding this comment

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

It seems strange for a static method to refer to its class by name...

Agreed

Maybe we could remove these three and use cls._float8_p3109_p_finfo(p) directly in __new__ below

Done. As an aside I put all the tests into the same for loop, makes the code rather tidier (and no measurable speed impact in pytest), hope that's reasonable.

@@ -411,4 +549,14 @@ def __new__(cls, dtype):
if _float8_e5m2fnuz_dtype not in cls._finfo_cache:
cls._finfo_cache[_float8_e5m2fnuz_dtype] = cls._float8_e5m2fnuz_finfo()
return cls._finfo_cache[_float8_e5m2fnuz_dtype]
for type_str, test_dtype, finfo in (
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's confusing that the local finfo variable shadows the finfo class name.

Copy link
Author

Choose a reason for hiding this comment

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

Totally agree, apologies.

@awf
Copy link
Author

awf commented Nov 11, 2023

@hawkinsp and I were discussing whether we might merge these new types in some sort of experimental/provisional way, to make clear that they might change. Maybe something like a ml_dtypes.provisional or ml_dtypes.experimental namespace? What do you think?

Your call, of course, although I do think the "p3109" name does some of that work for us - it's unlikely to be used "by accident".

Maybe a good choice might be between

  ml_dtypes.provisional.float8_ieee_p{P} # Easier to find-and-replace
  ml_dtypes.float8_p3109_p{P}

@jakevdp
Copy link
Collaborator

jakevdp commented Nov 12, 2023

Im not worried about users using these provisional types - I’m worried about downstream packages making releases which import them, and then if/when we remove those provisional names the packages will fail on import.

@jakevdp
Copy link
Collaborator

jakevdp commented Nov 12, 2023

All the float8 types have trailing strings of characters, I don’t think p3109 is very distinguishing in that sense.

@awf
Copy link
Author

awf commented Nov 13, 2023

I'm not worried about users using these provisional types - I’m worried about downstream packages making releases which import them, and then if/when we remove those provisional names the packages will fail on import.

Understood, so IMO this suggests inclusion of these types with the _p3109 suffix; then when future formal IEEE types arrive, say next year, we could include them with the _ieee suffix.

If it happens, as might be hoped, that they are the same types, we can simply alias

  float8_p3109_p3 = float8_ieee_p3
  # etc - perhaps 8-10 lines of aliases

If it happens that the future approved types are different, then, indeed we will end up with yet another set of types, and will need more of the genericization logic you've introduced with e.g. RequiresIsDerivedFromFloat8Base. I would be happy to do that work when/if needed, and incidentally have done some of it in this PR with finfo.__new__ and PyInit__ml_dtypes_ext.

@jakevdp
Copy link
Collaborator

jakevdp commented Nov 13, 2023

I don't think a _p3109 suffix communicates to potential users that the type is provisional and may be removed without warning from a future release. I think ml_dtypes.experimental.float8_p3109_p3 does a better job of communicating that.

If you want just a suffix, how about ml_dtypes.float8_p3_provisional_dont_depend_on_this ? I'm only half joking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants