Skip to content
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

Added base parameter to math.log #21151

Closed
wants to merge 1 commit into from

Conversation

Chillee
Copy link
Contributor

@Chillee Chillee commented May 30, 2019

Stack from ghstack:

Differential Revision: D15563185

@Chillee
Copy link
Contributor Author

Chillee commented May 30, 2019

Sorry for continually remaking this PR, but I think I messed up something small with ghstack that causes this PR to be constantly remade. Can someone stamp this so I can hopefully stop constantly remaking this PR?

@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label May 30, 2019
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.

Here might fail the lint check

@Chillee
Copy link
Contributor Author

Chillee commented May 30, 2019

It passes the precommit lint check - I think it's fine.

That code is also removed in a couple commits later in the stack.

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

@zou3519 zou3519 deleted the gh/chillee/12/head branch May 30, 2019 20:35
@xuhdev
Copy link
Collaborator

xuhdev commented May 30, 2019

What happens if b == 1? Perhaps we should at least add a test for that case?

@facebook-github-bot
Copy link
Contributor

@Chillee merged this pull request in 8002030.

@Chillee
Copy link
Contributor Author

Chillee commented May 30, 2019

@xuhdev I've done that later up in the stack (I unified all the math tests and added much more exhaustive checkign): #21125

But to answer your question, we return inf, which isn't ideal. Unluckily, there doesn't seem to be any easy way to handle these errors, so we've decided to just fail silently (+ inf is a natural value to return, although it doesn't conform exactly with the python API)

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.

5 participants