BUG: Fixing reduction logical ufuncs #220
Conversation
| inline npy_byte | ||
| from_quad<npy_byte>(quad_value x, QuadBackendType backend) | ||
| { | ||
| // reduction ops often give warning, we can handle the NAN casting |
There was a problem hiding this comment.
I don't really understand this comment
There was a problem hiding this comment.
I should improve this comment, basically saying the runtime warnings we used to get often casting is the reason.
Casting from quad NaN or inf to double, etc Instead if we just catch and return for example in this case if we catch NaN and return NP_True then there will be no warnings otherwise that part will go through casting and leads to warnings.
I tested this but revert to original with just comment. Often different ops use casting in one way or another so when fixing that, knowing this information might be helpful
Release table means? |
Oh the release tracker |
| } | ||
|
|
||
| PyObject *DTypes = PyTuple_Pack(3, &PyArrayDescr_Type, &PyArrayDescr_Type, &PyArray_BoolDType); | ||
| PyObject *DTypes = PyTuple_Pack(3, Py_None, Py_None, Py_None); |
There was a problem hiding this comment.
@seberg this has performance implications, right?
There was a problem hiding this comment.
It doesn't change things really. The problem with this (but also the old version) is mostly that it doesn't define an order...
... and I am not sure what happens if another DType wants to do this as well, they might clash (depending on who is faster).
In general, I think it may be best to just not worry about performance at all, since we cache promotion results. The problem then being just that there isn't really a well defined order for multiple similar promoters (which is probably fine in practice, except that I think NumPy rejects it for now).
(I.e. maybe for promoters it should be fine to say "yeah, just try it no matter what DTypes come in")
There was a problem hiding this comment.
@SwayamInSync ideally, you probably just register it twice, once with the first being quaddtype and once with the second.
Maybe we should explicitly allow this (for multiple dtypes, but then it has undefined order), though.
There was a problem hiding this comment.
ideally, you probably just register it twice
You mean registering promoter twice? I tried this regiserting the loop, it worked but undo because of code duplication.
I wanted the promoter to handle this stuff
|
Thanks @juntyr @ngoldbaum @seberg so I guess this seems fine. |
4 participants
closes #219