Closed brandonwillard closed 2 years ago
Merging #45 (65c7a37) into main (20611eb) will decrease coverage by
2.54%
. The diff coverage is97.59%
.:exclamation: Current head 65c7a37 differs from pull request most recent head 39ac1a5. Consider uploading reports for the commit 39ac1a5 to get more accurate results
@@ Coverage Diff @@
## main #45 +/- ##
==========================================
- Coverage 99.74% 97.20% -2.55%
==========================================
Files 7 9 +2
Lines 391 572 +181
Branches 31 62 +31
==========================================
+ Hits 390 556 +166
- Misses 0 5 +5
- Partials 1 11 +10
Impacted Files | Coverage Δ | |
---|---|---|
aemcmc/gibbs.py | 91.87% <93.05%> (-8.13%) |
:arrow_down: |
aemcmc/opt.py | 98.67% <98.67%> (ø) |
|
aemcmc/basic.py | 100.00% <100.00%> (ø) |
|
aemcmc/conjugates.py | 100.00% <100.00%> (ø) |
|
aemcmc/dists.py | 100.00% <100.00%> (ø) |
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 20611eb...39ac1a5. Read the comment docs.
We now have a complete working example in the test test_basic.py:test_create_gibbs
.
I've finished refactoring the sampler steps and filled out the docstings, so this should be ready to merge when/if it passes.
This PR provides an initial implementation of #3—and the related interface function mentioned in #26. It provides a general sampler-constructor,
aemcmc.basic.construct_sampler
, that returns adict
mappingRandomVariable
s to their sample steps.The current approach uses a
Feature
calledSamplerTracker
to track adict
fromRandomVariable
s to all their discovered sample steps—even when there's more than one potential sampler for the sameRandomVariable
. Sample steps are discovered by walking the graph with standard local rewriters that write their results to thedict
inSamplerTracker
. This allows us to maintain the original observation variable graphs in relation to every other un-observed variable (i.e. so we can see when a variable is in a particular hierarchical relationship with another variable, etc.)In order to get around some
DimShuffle
annoyances during unification/pattern-matching, aSubsumingElemwise
Op
was added and is used to replaceElemwise(DimShuffle(x), ...)
graphs withSubsumingElemwise(x, ...)
graphs (i.e. ones that subsume theDimShuffle
s). SinceSubsumingElemwise
inherits fromOpFromGraph
, those nodes can be expanded later on to reproduce the originalElemwise
+DimShuffle
sub-graphs.RandomVariable
s and construct samplers for them. While doing so, it needs to use references to the previously constructed samplers' outputs.RandomVariable
rewriting The issue here is thatRandomVariable
s that are canonicalized are no longer the sameRandomVariable
s that the user created, so we need a means of keeping a map between the two. LiftingOp
s throughRandomVariable
s is one of the main ways this issue shows up. N.B.: This is also one situation in which we could use complete relations (i.e. two-way rewrites).DimShuffle
sSubsumingElemwise
will subsume (i.e. limit to only ones that add the appropriate broadcast dimensions). In cases where the original graph was aElemwise(DimShuffle1(DimShuffle2(x)), ...)
and the twoDimShuffle*
are merged, we will need to un-merge/expand them in order to useSubsumingElemwise
.RandomVariable
s).aemcmc.gibbs
(e.g. remove combination samplers in favor ofconstruct_sampler
, create morelocal_optimizer
s, generalizelocal_optimizer
construction, etc.)