BUG,ENH: Relax finfo to be easier accessible for all user dtypes#31563
BUG,ENH: Relax finfo to be easier accessible for all user dtypes#31563seberg wants to merge 2 commits into
Conversation
This does two small tweaks:
1. Remove the requirement for `inexact` scalar derivation (because I had
not realized that this is problematic for bfloat16, etc.).
* This makes the error message very slightly nicer for non-float
finfo inputs, I expect that is OK.
(This means we require all finfo fields to be filled in right now
which quaddtype does as the only user, we could omit the decimal
precision or so if someone asks for it.)
2. For complex dtypes rather than hard-code ours, assume that `arr.imag`
succeeds and use `arr.imag.dtype` when it is a view to find the "real"
dtype.
After that, we assume that `finfo` filling will succeed.
Together (with the "new dtype backporting") this will allow `ml_dtypes`
to support `np.finfo()` fully.
(It is very annoying to test it here, but we run `ml_dtypes` tests now
and they will test it as soon as we start using it there.)
| try: | ||
| arr = numeric.empty(1, dtype=dtype) | ||
| imag_part = arr.imag | ||
| real_part = arr.real |
There was a problem hiding this comment.
To be clear, my first plan for this function was to guard it with the AbstractDType hierarchy, but since that is stalled behind heap-types, etc. and the .imag path is good and used either way, that guard felt a bit unnecessary.
(ml_dtypes cannot inherit from the scalar hierarchy easily, I think because it would mean inheriting the scalar methods and they make wrong assumptions.)
Yes, no problem, we return 1 for all six now-mandatory float constants (and the four int ones), so quaddtype is unaffected. |
I didn't thought about it earlier but yeah that logic routes the input to default scalar type. I think we can leave |
|
Actually, pushed to use |
3 participants
This does two small tweaks:
inexactscalar derivation (because I had not realized that this is problematic for bfloat16, etc.).arr.imagsucceeds and usearr.imag.dtypewhen it is a view to find the "real" dtype. After that, we assume thatfinfofilling will succeed.Together (with the "new dtype backporting") this will allow
ml_dtypesto supportnp.finfo()fully.(It is very annoying to test it here, but we run
ml_dtypestests now and they will test it as soon as we start using it there.)(No AI use.)
CC @SwayamInSync since you were involved in the first part. I don't think requiring all "finfo" fields that we currently have in C to be filled should be a problem, right?
(If that changes, there is always the work-around to fill it with NaN in practice, probably.)
(I find the
obj2sctypeannoying and potentially fishy even for quaddtype since it is slightly parametric(?), but that part I don't dare to touch here.)FWIW, I would like to consider this a bug-fix and backport. The only change should be the error message change in practice.