-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
ENH: Add smallest_normal and smallest_subnormal attributes to finfo #18536
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
Conversation
Thanks @steff456! Could you also make the corresponding change to To add to the rationale: when looking into which attributes of The |
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.
Looks good, just needs a few tweaks, in addition to updating machar.py
a7a2703
to
b34fda5
Compare
There are failures for Two suggestions:
|
Two questions:
Are these taken into account? |
I just checked and As Chuck mentioned, it is plausible that this value might end up
So this seems unproblematic if we use We should probably bikeshed the name on the mailing list, its annoying that python uses |
Here are other ways to compute the smallest subnormal numbers:
I have not tested if the above is suitable for float128: there is no int128, etc. |
Can this be the correct value for float128:
?
|
Please, lets not cook our own soup here? Just implement the |
I think in here you are calculating the |
I am not sure that this would be completely our own soup here. See for instance https://en.wikipedia.org/wiki/Single-precision_floating-point_format which shows the smallest positive number of float32:
The same pattern applies to float64 and apparently to float128 as well:
I think it would be interesting to see if the |
seems to work fine. |
So it sounds like this is converging.
Note that for (1) we should keep in mind that
I agree with the best choice. Let's just do the mailing list message but avoid the bikeshedding. |
In the else block,
|
Before anyone gets more confused... Its not called |
I assumed that we are talking about float128 (or binary128) as described in https://en.wikipedia.org/wiki/Quadruple-precision_floating-point_format . It would be strange to call |
No @pearu we are most definitely not, there is no such thing in NumPy, float128 is just a badly named alias for |
@seberg good point, but I'm not sure it applies to I don't have a machine at hand where
Whether |
Also note that this is a somewhat implementation-centric view, that's probably good for downstream library devs who simply want to support all dtypes - but not very useful for end users who need extended precision. The only good reason to be using any of the extended precision dtypes in application code is if 64 bits of precision is not enough. The last thing you'd then want is to use |
Of course it also applies to That the code here lists all types of possible I tried to nudge this thing in direction of using
The whole discussion mentioning |
Just FYI. In CuPy we turn flush-to-zero on by default (via setting a compiler flag |
Needs release note. |
Needs a release note. |
The latest commit includes the change to use As well, I added comments to add a disclaimer that the value of the |
xref gh-8504 which is the PR adding this originally. I honestly don't know if this is right or not. @charris or even @matthew-brett do you have any insight for the smallest subnormal? In general I would prefer using nextafter everywhere and unconditionally. Note that the There seems to be the problem that |
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 made some last comments, the rest looks fine. I still expect the NaN return won't be great, but since the old value was probably wrong, might as well.
Maybe one of the other early reviewers wants to finalize?
7a65ca9
to
7610635
Compare
7610635
to
8ad28c4
Compare
@steff456 there are merge conflicts. |
Ok, give this a shot. Thanks everyone! |
Reflect the `finfo` changes, made in numpy#18536, in the classes' type annotations.
Reflect the `finfo` changes, made in numpy#18536, in the classes' type annotations.
This was blocked on numpy#18536, which has been merged.
Reflect the `finfo` changes, made in numpy#18536, in the classes' type annotations.
This fixes the missing attributes. I tested the warning and fix on valgrind itself. These attributes were added in numpygh-18536 but the fallback path was not checked there. Replaces numpygh-21813, although something more like it may make sense if it allows us to just delete MachAr completely.
This fixes the missing attributes. I tested the warning and fix on valgrind itself. These attributes were added in numpygh-18536 but the fallback path was not checked there. Replaces numpygh-21813, although something more like it may make sense if it allows us to just delete MachAr completely.
Given the discussions happened in data-apis/array-api#129 of the Data API consortium we decided to create a PR to add
smallest_normal
andsmallest_subnormal
numbers to finfo object. All the values added were checked against IEEE-754 and a test was included.cc @rgommers @kgryte