-
-
Notifications
You must be signed in to change notification settings - Fork 10
np.finfo
support for quaddtype
#159
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
base: main
Are you sure you want to change the base?
Conversation
@juntyr can you please take a look at the |
static PyObject * | ||
QuadPrecDType_finfo(QuadPrecDTypeObject *self, PyObject *args) | ||
{ | ||
PyObject *numpy_quaddtype_module = PyImport_ImportModule("numpy_quaddtype"); |
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 don't like doing this as well, but for now it works so
eps: float = float(epsilon) | ||
epsneg: float = float(epsilon) | ||
iexp: int = int(precision) | ||
machar: object = None |
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.
What is this property?
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.
is just an object finfo uses to fetch the finfo properties for dtypes, since we are defining them in our package hence marking it as None
""" | ||
bits: int = int(bits) | ||
eps: float = float(epsilon) | ||
epsneg: float = float(epsilon) |
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.
This is wrong - it should 1 - nextafter(1, 0) [similar to how exp is nextafter(1, 2) - 1], we can probably compute this in the C++ source and then expose it as a new constant
bits: int = int(bits) | ||
eps: float = float(epsilon) | ||
epsneg: float = float(epsilon) | ||
iexp: int = int(precision) |
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.
iexp should be 15, I think?
epsneg: float = float(epsilon) | ||
iexp: int = int(precision) | ||
machar: object = None | ||
machep: float = float(epsilon) |
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.
machep should be int(log2(epsilon))?
machar: object = None | ||
machep: float = float(epsilon) | ||
max: float = float(max_value) | ||
maxexp: float = float(max_value) |
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.
This would probably be int(ceil(max_value))?
machep: float = float(epsilon) | ||
max: float = float(max_value) | ||
maxexp: float = float(max_value) | ||
min: float = float(smallest_normal) |
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.
min = -max_value
max: float = float(max_value) | ||
maxexp: float = float(max_value) | ||
min: float = float(smallest_normal) | ||
minexp: float = float(smallest_normal) |
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.
not sure how to compute this one
maxexp: float = float(max_value) | ||
min: float = float(smallest_normal) | ||
minexp: float = float(smallest_normal) | ||
negep: float = float(epsilon) |
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.
int(log2(epsneg))
min: float = float(smallest_normal) | ||
minexp: float = float(smallest_normal) | ||
negep: float = float(epsilon) | ||
nexp: int = int(bits) - int(precision) - 1 |
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.
maybe 15 + 1? not sure
minexp: float = float(smallest_normal) | ||
negep: float = float(epsilon) | ||
nexp: int = int(bits) - int(precision) - 1 | ||
nmant: int = int(precision) |
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.
113 or 112
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 @SwayamInSync for working on this!
I am really uncomfortable with all the casts to float, since at least for some of them they will undoubtedly over- or underflow. In my mind, the finfo values should have the same dtype as the type we're describing, but I know this would likely be a breaking change; but likely one we need since otherwise e.g. finfo(quad).max don't make any sense
Hey yes good point, just fixed that |
What's the reason for using the explicit field constructor? |
np.finfo
support for quaddtyoenp.finfo
support for quaddtype
Making this PR here as well, as a demonstration to how
np.finfo
and other related information can be passed to NumPy.Edit: Below is not possible as
QuadPrecDType
is an immutable typeBut this just feels so illegal that's why registering as
.tp_methods
From the NumPy side it requires numpy/numpy#29771 to get merged.