-
Notifications
You must be signed in to change notification settings - Fork 26.3k
Added better tests for math ops and unified them #21125
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
Added better tests for math ops and unified them gh-metadata: pytorch pytorch 21125 gh/chillee/3/head
Added better tests for math ops and unified them gh-metadata: pytorch pytorch 21125 gh/chillee/3/head
Added better tests for math ops and unified them gh-metadata: pytorch pytorch 21125 gh/chillee/3/head
Added better tests for math ops and unified them gh-metadata: pytorch pytorch 21125 gh/chillee/3/head
Added better tests for math ops and unified them gh-metadata: pytorch pytorch 21125 gh/chillee/3/head
Added better tests for math ops and unified them gh-metadata: pytorch pytorch 21125 gh/chillee/3/head
Added better tests for math ops and unified them gh-metadata: pytorch pytorch 21125 gh/chillee/3/head
Added better tests for math ops and unified them gh-metadata: pytorch pytorch 21125 gh/chillee/3/head
Added better tests for math ops and unified them gh-metadata: pytorch pytorch 21125 gh/chillee/3/head
test/test_jit.py
Outdated
| return math.log10(x) | ||
| inf = float("inf") | ||
| NaN = float("nan") | ||
| mx_int = 2147483647 |
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.
we use int64_t internally so we should try be 2 ** 31, 2 ** 32, and 2 ** 63 (and the negations)
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 do the first 2 - I'll add the last one.
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.
Actually, I don't think it's a good idea. Python has big ints, so it'll invariably have different behavior than what we do near the boundary.
Added better tests for math ops and unified them gh-metadata: pytorch pytorch 21125 gh/chillee/3/head
Added better tests for math ops and unified them gh-metadata: pytorch pytorch 21125 gh/chillee/3/head
| .format(func_name=func_name, a=a, b=b, resf=resf, resfs=resfs)) | ||
|
|
||
|
|
||
| unary_float_ops = ["log", "log1p", "log10", "exp", "sqrt", "gamma", "lgamma", "erf", "erfc", "expm1", "fabs"] |
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.
We should have something like [x for x in dir(math) if callable(getattr(math, x))] so we know we're getting full coverage of the math module instead of maintaining big lists like these (though we still need some list to say what the args are for the non-unary ones)
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'd prefer to do this after this stack lol - otherwise I'd need to maintain a list of what functions aren't supported that I whittle down for each commit in this stack.
Added better tests for math ops and unified them gh-metadata: pytorch pytorch 21125 gh/chillee/3/head
Added better tests for math ops and unified them gh-metadata: pytorch pytorch 21125 gh/chillee/3/head
ailzhang
left a comment
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.
LGTM!
Added better tests for math ops and unified them gh-metadata: pytorch pytorch 21125 gh/chillee/3/head
Added better tests for math ops and unified them gh-metadata: pytorch pytorch 21125 gh/chillee/3/head
Stack from ghstack:
Differential Revision: D15563188