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.

Sign up for GitHub

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

Jump to bottom

Rewrote bayes_risk to allow multiple expparams #134

Merged

ihincks merged 14 commits into QInfer:master from ihincks:upgrade-vectorized-risk

Aug 17, 2018

Collaborator

ihincks commented Jun 7, 2017

It would be nice for bayes_risk to vectorize over inputs to make use of any speed-ups that the likelihood might have along its expparams axis. This PR is a stab at that. It seems to be working for the model I am working on, but needs a bit more testing.


          rewrote bayes_risk to allow multiple expparams

78db669

coveralls commented Jun 7, 2017 •

edited

Loading

Coverage decreased (-0.07%) to 74.114% when pulling 78db669 on ihincks:upgrade-vectorized-risk into 41ddbad on QInfer:master.


          vectorized expected_information_gain as well

fe5a158

coveralls commented Jun 12, 2017

Coverage decreased (-0.2%) to 74.014% when pulling fe5a158 on ihincks:upgrade-vectorized-risk into 41ddbad on QInfer:master.

3 similar comments

coveralls commented Jun 12, 2017

Coverage decreased (-0.2%) to 74.014% when pulling fe5a158 on ihincks:upgrade-vectorized-risk into 41ddbad on QInfer:master.

coveralls commented Jun 12, 2017

Coverage decreased (-0.2%) to 74.014% when pulling fe5a158 on ihincks:upgrade-vectorized-risk into 41ddbad on QInfer:master.

coveralls commented Jun 12, 2017

Coverage decreased (-0.2%) to 74.014% when pulling fe5a158 on ihincks:upgrade-vectorized-risk into 41ddbad on QInfer:master.

ihincks added 4 commits

June 13, 2017 09:36


          made DifferentiableBinomialModel visible to __all__

8af9f97


          fixed a bugA

6c9df98


          added some tests for risk and info gain

a0e4dbd


          indexing bug fix for vectorized functions

add8e73

coveralls commented Jun 13, 2017 •

edited

Loading

Coverage increased (+1.0%) to 75.157% when pulling add8e73 on ihincks:upgrade-vectorized-risk into 41ddbad on QInfer:master.

coveralls commented Jun 13, 2017

Coverage increased (+1.0%) to 75.157% when pulling add8e73 on ihincks:upgrade-vectorized-risk into 41ddbad on QInfer:master.

3 similar comments

coveralls commented Jun 13, 2017

Coverage increased (+1.0%) to 75.157% when pulling add8e73 on ihincks:upgrade-vectorized-risk into 41ddbad on QInfer:master.

coveralls commented Jun 13, 2017

Coverage increased (+1.0%) to 75.157% when pulling add8e73 on ihincks:upgrade-vectorized-risk into 41ddbad on QInfer:master.

coveralls commented Jun 13, 2017

Coverage increased (+1.0%) to 75.157% when pulling add8e73 on ihincks:upgrade-vectorized-risk into 41ddbad on QInfer:master.

Collaborator Author

ihincks commented Jun 13, 2017

I'm done with what I wanted to do in this PR (aside from doc build error fix). Thanks to @whitewhim2718 and his feature-generalized-outcomes branch from which I copied some ideas and code.

As usual, I find the doc error messages pretty cryptic, and don't know where to start at the moment.

ihincks requested a review from cgranade

June 13, 2017 19:03

Collaborator

scasagrande commented Jun 14, 2017

I took a look at your error log. All the red lines seem to be warnings. I'm pretty sure the error failing the build is make[1]: latexmk: Command not found on line 1564 (https://travis-ci.org/QInfer/python-qinfer/jobs/242525398#L1564).

Collaborator

scasagrande commented Jun 14, 2017

The last master build does this:

Running LaTeX files through pdflatex...
$ make -C _build/latex all-pdf
make[1]: Entering directory `/home/travis/build/QInfer/python-qinfer/doc/_build/latex'
$ pdflatex  'QInfer.tex'

And your build is doing this:

Running LaTeX files through pdflatex...
$ make -C _build/latex all-pdf
make[1]: Entering directory `/home/travis/build/QInfer/python-qinfer/doc/_build/latex'
$ latexmk -pdf -dvi- -ps-  'QInfer.tex'

Taking a look at the installed dependencies, I see master installed sphinx==1.5.6 and your build installed sphinx==1.6.2, which might be responsible.

Collaborator Author

ihincks commented Jun 14, 2017

Thanks for looking into this, @scasagrande . It seems that they switched to latexmk recently: sphinx-doc/sphinx#3543


          added latexmk to travis build

00e8366

Collaborator Author

ihincks commented Jun 14, 2017

Bah, need to somehow turn off interactive mode in latexmk, the build has stalled. Either that, or revert to an older version of sphinx, which is probably not a great idea.

coveralls commented Jun 14, 2017 •

edited

Loading

Coverage increased (+1.0%) to 75.157% when pulling 00e8366 on ihincks:upgrade-vectorized-risk into 41ddbad on QInfer:master.

1 similar comment

coveralls commented Jun 14, 2017

Coverage increased (+1.0%) to 75.157% when pulling 00e8366 on ihincks:upgrade-vectorized-risk into 41ddbad on QInfer:master.


          added a request for pdflatex nonstop mode, not sure if this will work

7fcc279

coveralls commented Jun 14, 2017

Coverage increased (+1.0%) to 75.157% when pulling 7fcc279 on ihincks:upgrade-vectorized-risk into 41ddbad on QInfer:master.

2 similar comments

coveralls commented Jun 14, 2017

Coverage increased (+1.0%) to 75.157% when pulling 7fcc279 on ihincks:upgrade-vectorized-risk into 41ddbad on QInfer:master.

coveralls commented Jun 14, 2017

Coverage increased (+1.0%) to 75.157% when pulling 7fcc279 on ihincks:upgrade-vectorized-risk into 41ddbad on QInfer:master.

coveralls commented Jun 14, 2017 •

edited

Loading

Coverage increased (+5.4%) to 79.905% when pulling b9bd357 on ihincks:upgrade-vectorized-risk into 3c9cc7e on QInfer:master.

Collaborator

scasagrande commented Jun 14, 2017

I'd lock it to the older version for now, and then make it a separate PR to update to the latest.

cgranade approved these changes

View reviewed changes

Collaborator

cgranade left a comment

Thanks for this, it looks great! I apologize as always for the delay, but I'm happy to merge as soon as we figure out the LaTeX errors. I'll take a stab at that, cherry picking off your improvements to .travis.yml in a separate PR.

src/qinfer/smc.py Outdated

+                      # compute the hypothetical weights, likelihoods and normalizations for
+                      # every possible outcome and expparam
+                      # the likelihood over outcomes should sum to 1, so don't compute for last outcome
+                      w_hyp, L, N = self.hypothetical_update(

Collaborator

cgranade Jul 3, 2017

I really like this optimization, it should help a lot with models that have a small number of outcomes. Since the same optimization appears in expected_information_gain, perhaps it's worth abstracting / refactoring it out?

Collaborator

cgranade commented Jul 3, 2017

Looking at the Travis build failures, it seems as though the latex_preamble Sphinx configuration key is not being propagated correctly in the newest version. It appears that the key is now latex_elements["preamble"] as of 1.5.1, so it might work to provide the preamble in both forms. I'll try that and PR it now.

cgranade mentioned this pull request

Fix Sphinx LaTeX builds #135

Merged

ihincks and others added 5 commits

August 14, 2017 09:08


          small bug fix

29b72ea


          Merge branch 'bugfix-resampler-weight-shape' into upgrade-vectorized-…

297b3e5

…risk


          changed bayes_risk and expected_info_gain to use model domain

a02c055


          fixed serial mode bug

ebdec57


          Merge branch 'master' into upgrade-vectorized-risk

69b5c0e

Collaborator

cgranade commented Aug 9, 2018

This should now work with #135 merged in. Resolved conflicts and re-ran tests.


          fixed bug when is_outcomes_constant is False

b9bd357

ihincks merged commit 8170c84 into QInfer:master

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

Labels

None yet