Add nan_to_num helper
#796
Conversation
Dhruvanshu-Joshi
force-pushed
the
nan
branch
2 times, most recently
from
5081464 to
f97146c
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #796 +/- ##
==========================================
+ Coverage 80.76% 81.01% +0.24%
==========================================
Files 163 170 +7
Lines 46941 46941
Branches 11499 11499
==========================================
+ Hits 37912 38029 +117
+ Misses 6786 6705 -81
+ Partials 2243 2207 -36
|
pytensor/scalar/basic.py
Outdated
| isposinf = IsPosInf() | ||
|
|
||
|
|
||
| class IsNegInf(FixedLogicalComparison): |
There was a problem hiding this comment.
Do we need these custom Ops, or can we just use helper functions like nan_to_num such as pt.eq(x, -np.inf)?
In general we want to have as little Ops as possible, and just reuse what we already have.
There was a problem hiding this comment.
I just referred to this conversation and implemented the two ops. This can be done even without them. Should I omit these in the next commit?
There was a problem hiding this comment.
Yup, no new Ops is always the default and preferred strategy
pytensor/tensor/math.py
Outdated
| is_pos_inf = eq(x, np.inf) | ||
| is_neg_inf = eq(x, -np.inf) |
There was a problem hiding this comment.
Can also add the numpy helpers for isneginf, isposinf for users (and use them here)
There was a problem hiding this comment.
I created Isposinf and Isneginf with the very same purpose here:
Dhruvanshu-Joshi@829309b
How can we use np.isposinf(x) directly in helper functions without creating an op when x is but a container?
There was a problem hiding this comment.
You don't, you use the pt.isponsinf helper which will have this exact code inside pt.eq(pt.as_tensor(x), np.inf).
|
The new test is failing in the floatX="float32" runs (happens all the time and it's super annoying): https://github.com/pymc-devs/pytensor/actions/runs/9603508611/job/26486909276?pr=796 |
Added the |
tests/tensor/test_math.py
Outdated
|
|
||
| f = function([x], out, allow_input_downcast=True) | ||
|
|
||
| y = np.array([1, 2, np.nan, np.inf, -np.inf, 3, 4]) |
There was a problem hiding this comment.
This would solve the failing float32 test without having to downcast the input
| y = np.array([1, 2, np.nan, np.inf, -np.inf, 3, 4]) | |
| y = np.array([1, 2, np.nan, np.inf, -np.inf, 3, 4]).astype(x.dtype) |
|
Is this ready to merge? |
Description
Added pytensor equivalent of numpy's nan_to_num. Also added
isposinfandisneginfsimilar toisinf.Related Issue
numpy.nan_to_num#479Checklist
Type of change