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

Evaluate only desired branch in log1mexp_numpy #4428

Merged
merged 3 commits into from
Jan 22, 2021

Conversation

ricardoV94
Copy link
Member

@ricardoV94 ricardoV94 commented Jan 21, 2021

This PR changes log1mexp_numpy in the math module to only apply one of the two operations to the inputs. This avoids the non-important but common RuntimeWarning during NUTS sampling when logdiffexp_numpy is called in the stats method:

RuntimeWarning: divide by zero encountered in log1p
return np.where(x < 0.6931471805599453, np.log(-np.expm1(-x)), np.log1p(-np.exp(-x)))

The RuntimeWarning occurred because both operations were applied to x before selecting the desired (more numerical stable) output with np.where.

I added a test that fails in Master but passes in this PR.

@ricardoV94 ricardoV94 marked this pull request as draft January 21, 2021 15:56
@ricardoV94
Copy link
Member Author

Maybe this shouldn't change x in place...

@michaelosthege michaelosthege added this to the vNext milestone Jan 21, 2021
@ricardoV94 ricardoV94 marked this pull request as ready for review January 21, 2021 17:33
@ricardoV94
Copy link
Member Author

Also, if the new unittests feel like too much, I am happy to remove them.

Copy link
Member

@michaelosthege michaelosthege left a comment

Choose a reason for hiding this comment

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

Looks good now. The tests are fine too.

I think you should add a line to the release notes - I also added this to a "vNext (3.11.1)" milestone.

@ricardoV94
Copy link
Member Author

Added. I also had to create the new "vNext (on deck)" section.

@ricardoV94 ricardoV94 force-pushed the hide_warning_log1mexp_numpy branch from e908893 to c712fdb Compare January 22, 2021 17:16
@ricardoV94 ricardoV94 merged commit 823906a into pymc-devs:master Jan 22, 2021
@ricardoV94 ricardoV94 deleted the hide_warning_log1mexp_numpy branch January 22, 2021 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants