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

Multivariate floating point precision fix #2470

Merged
merged 14 commits into from Aug 3, 2017

Conversation

Projects
None yet
3 participants
@ericmjl
Contributor

ericmjl commented Aug 2, 2017

This is a bug fix for distributions.multivariate's Multinomial errors related to floating point precision. This will very likely be helpful for computation runs that involve the GPU, and involve many classes with small floating point numbers.

More information can be found here: #2469

@ericmjl

This comment has been minimized.

Show comment
Hide comment
@ericmjl

ericmjl Aug 2, 2017

Contributor

Just saw a test fail, I think I know what's going on. Let me fix that.

Contributor

ericmjl commented Aug 2, 2017

Just saw a test fail, I think I know what's going on. Let me fix that.

@ericmjl

This comment has been minimized.

Show comment
Hide comment
@ericmjl

ericmjl Aug 2, 2017

Contributor

Okay, locally tests seem like they're passing.

$  py.test pymc3/tests/test_distributions_random.py
============================ test session starts ============================
platform linux -- Python 3.6.0, pytest-3.0.5, py-1.4.32, pluggy-0.4.0
rootdir: /home/ericmjl/github/software/pymc3, inifile: setup.cfg
collected 214 items

pymc3/tests/test_distributions_random.py ....................................
.............................................................................
.............................................................................
.....................xss

============ 211 passed, 2 skipped, 1 xfailed in 253.00 seconds =============

$ py.test pymc3/tests/test_distributions.py
============================ test session starts ============================
platform linux -- Python 3.6.0, pytest-3.0.5, py-1.4.32, pluggy-0.4.0
rootdir: /home/ericmjl/github/software/pymc3, inifile: setup.cfg
collected 95 items

pymc3/tests/test_distributions.py ....................................x...x..
...xxx...x.....................................x....

=================== 88 passed, 7 xfailed in 55.32 seconds ===================

I'm feeling a bit more confident now that the tests on Travis should pass.

Contributor

ericmjl commented Aug 2, 2017

Okay, locally tests seem like they're passing.

$  py.test pymc3/tests/test_distributions_random.py
============================ test session starts ============================
platform linux -- Python 3.6.0, pytest-3.0.5, py-1.4.32, pluggy-0.4.0
rootdir: /home/ericmjl/github/software/pymc3, inifile: setup.cfg
collected 214 items

pymc3/tests/test_distributions_random.py ....................................
.............................................................................
.............................................................................
.....................xss

============ 211 passed, 2 skipped, 1 xfailed in 253.00 seconds =============

$ py.test pymc3/tests/test_distributions.py
============================ test session starts ============================
platform linux -- Python 3.6.0, pytest-3.0.5, py-1.4.32, pluggy-0.4.0
rootdir: /home/ericmjl/github/software/pymc3, inifile: setup.cfg
collected 95 items

pymc3/tests/test_distributions.py ....................................x...x..
...xxx...x.....................................x....

=================== 88 passed, 7 xfailed in 55.32 seconds ===================

I'm feeling a bit more confident now that the tests on Travis should pass.

@ericmjl

This comment has been minimized.

Show comment
Hide comment
@ericmjl

ericmjl Aug 2, 2017

Contributor

@ColCarroll @junpenglao all tests pass!!!!!!

Contributor

ericmjl commented Aug 2, 2017

@ColCarroll @junpenglao all tests pass!!!!!!

@ColCarroll

looks good to merge now -- had a few small comments if you have a second. excited to try this out and see if it fixes my problems!

Show outdated Hide outdated pymc3/distributions/multivariate.py
Show outdated Hide outdated pymc3/distributions/multivariate.py
Show outdated Hide outdated pymc3/distributions/multivariate.py
@ericmjl

This comment has been minimized.

Show comment
Hide comment
@ericmjl

ericmjl Aug 2, 2017

Contributor

@ColCarroll I ran the two relevant tests locally and found that they pass. I'm confident it'll pass on Travis, but as usual, let's wait it out.

Contributor

ericmjl commented Aug 2, 2017

@ColCarroll I ran the two relevant tests locally and found that they pass. I'm confident it'll pass on Travis, but as usual, let's wait it out.

@ericmjl

This comment has been minimized.

Show comment
Hide comment
@ericmjl

ericmjl Aug 2, 2017

Contributor

@ColCarroll @junpenglao @kyleabeauchamp I think one of the tests timed out, is that correct? Otherwise, everything else has passed.

Contributor

ericmjl commented Aug 2, 2017

@ColCarroll @junpenglao @kyleabeauchamp I think one of the tests timed out, is that correct? Otherwise, everything else has passed.

@ColCarroll

This comment has been minimized.

Show comment
Hide comment
@ColCarroll

ColCarroll Aug 3, 2017

Member

Yeah, one is a flaky test, one looks like coveralls threw a 5xx error. I restarted both.

Member

ColCarroll commented Aug 3, 2017

Yeah, one is a flaky test, one looks like coveralls threw a 5xx error. I restarted both.

@ColCarroll ColCarroll merged commit 259613f into pymc-devs:master Aug 3, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.003%) to 86.681%
Details
@ColCarroll

This comment has been minimized.

Show comment
Hide comment
@ColCarroll

ColCarroll Aug 3, 2017

Member

Thanks for fixing this!

Member

ColCarroll commented Aug 3, 2017

Thanks for fixing this!

@ericmjl

This comment has been minimized.

Show comment
Hide comment
@ericmjl

ericmjl Aug 3, 2017

Contributor

Yay!!!! Thanks @ColCarroll!

Contributor

ericmjl commented Aug 3, 2017

Yay!!!! Thanks @ColCarroll!

@ericmjl ericmjl deleted the ericmjl:multivariate_fix branch Aug 3, 2017

denadai2 added a commit to denadai2/pymc3 that referenced this pull request Aug 9, 2017

Multivariate floating point precision fix (#2470)
* fixed multivariate

* multivariate fix for pvals > 1

* updated multivariate with hack

* bugfix

* float64 patch to multivariate's multinomial

* reinstated p.squeeze(), accidentally took it out

* updated based on failed test

* further minor fixes to multinomial

* Force push to restart Travis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment