Skip to content

Conversation

@Chillee
Copy link
Collaborator

@Chillee Chillee commented May 30, 2019

Stack from ghstack:

Differential Revision: D15563188

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
Chillee added 3 commits May 30, 2019 21:13
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
Copy link
Contributor

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)

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

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.

Chillee added 2 commits May 31, 2019 11:12
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"]
Copy link
Contributor

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)

Copy link
Collaborator Author

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.

Chillee added 2 commits May 31, 2019 12:48
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
Copy link
Contributor

@ailzhang ailzhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Chillee added 2 commits June 4, 2019 11:53
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
@zou3519 zou3519 deleted the gh/chillee/3/head branch June 4, 2019 20:18
@facebook-github-bot
Copy link
Contributor

@Chillee merged this pull request in 2ed6f01.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merged oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants