jeff-regier / Celeste.jl

Scalable inference for a generative model of astronomical images
MIT License
183 stars 28 forks source link

fix test comparing joint and single infer #726

Closed kbarbary closed 6 years ago

kbarbary commented 6 years ago

Currently there isn't actually a test of joint inference. It had been tested in the long-running tests, but these have become quite out-of-date and thoroughly broken. This PR rescues one of those tests and leaves the rest of the long-running tests in a file test/outofdate.jl. (It is not clear how to fix these or whether they are really necessary and it's not currently a priority.)

The rescued test has been moved to test_infer.jl and is now always run. It runs joint VI and single VI on a few overlapping sources and checks that the joint result is better.

There are also a few whitespace changes and minor cleanups in ParallelRun.jl.

codecov[bot] commented 6 years ago

Codecov Report

Merging #726 into master will increase coverage by 0.01%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #726      +/-   ##
=========================================
+ Coverage   81.49%   81.5%   +0.01%     
=========================================
  Files          38      38              
  Lines        3934    3937       +3     
=========================================
+ Hits         3206    3209       +3     
  Misses        728     728
Impacted Files Coverage Δ
src/ParallelRun.jl 85.87% <100%> (+0.16%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b342a76...5f47872. Read the comment docs.

jeff-regier commented 6 years ago

When possible, better to treat the joint inference code and constraint transforms code as black boxes. But perhaps some changes to them can't be avoided to run on DECaLS.

kbarbary commented 6 years ago

When possible, better to treat the joint inference code and constraint transforms code as black boxes.

Yes, it would be great to be able to do this. But the tests which need to be updated use many internals, so there is no avoiding looking at the internals in order to get the tests running.

Beyond the scope of my current project, I can imagine having a better "internal API" for ELBO. One that allows it to be treated more as a black box that can provide the objective function and gradient in single function calls.