-
Notifications
You must be signed in to change notification settings - Fork 29
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
base: main
Are you sure you want to change the base?
Conversation
@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. |
A couple of quick questions:
|
These recommendations represent the discussions so far of the committee. The major decisions:
Have been formally voted on, and current work is on defining operations on these types. Regarding final names, once a standard is written, I would expect the
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: |
I have tested global find and replace of |
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.
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?
ml_dtypes/_finfo.py
Outdated
|
||
@staticmethod | ||
def _float8_p3109_p5_finfo(): | ||
return finfo._float8_p3109_p_finfo(5) |
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 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
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 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.
ml_dtypes/_finfo.py
Outdated
@@ -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 ( |
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's confusing that the local finfo
variable shadows the finfo
class name.
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.
Totally agree, apologies.
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} |
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. |
All the float8 types have trailing strings of characters, I don’t think p3109 is very distinguishing in that sense. |
Understood, so IMO this suggests inclusion of these types with the 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. |
I don't think a If you want just a suffix, how about |
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)
p
as in IEEE-754 (p
includes the implicit leading mantisa bit so exponent widthw
isw = 8-p
)emax = 2^(w-1)-1
, following IEEE-754float8_p3109_p{p}
in Python, templatedfloat8_p3109_p<p>
in C++See Colab:ML Dtypes.ipynb to experiment with this branch.