Ancestral Sampling and Bayes Ball Algorithm #233
Conversation
…st for ancestral sampling
Pull Request Test Coverage Report for Build 11937874024Details
💛 - Coveralls |
|
BridgeStan not found at location specified by $BRIDGESTAN environment variable, downloading version 2.5.0 to /home/runner/.bridgestan/bridgestan-2.5.0
|
|
thanks @naseweisssss, could you resolve the formatter's complaints, also maybe we should have a slightly more complicated model for the test? also, it'll be helpful to run the ancestral sampling multiple time and look at the distributions of the variables. |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
linting Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
There was a problem hiding this comment.
Sorry for the late review, in general this is very good job! Thanks
I left some comments, but all minor.
A suggestion: next time it'll be better to add some descriptions, it doesn't just help me understand the algorithms, but also explain why you might choose some methods over others. You can also factor a lot of these writings in future reports.
Particularly for implementation of algorithm, a small example would help a lot, giving some correctness proof (sketches are fine) would be great. Doing this kind of writing is something I didn't do very well at, but it is worthwhile to spent some time on.
| return true | ||
| end | ||
|
|
||
| function has_conditioned_descendant(bn::BayesianNetwork, node_id::Int, z_ids::Set{Int}) |
There was a problem hiding this comment.
slight confusion here, why the ball can pass through if the collider child has conditioned descendants? and this could also incur repeated computations?
There was a problem hiding this comment.
https://pmc.ncbi.nlm.nih.gov/articles/PMC6089543/figure/F4/
I am considering this. Please let me know what do you think.
There was a problem hiding this comment.
Yes I think it might incur repeated computations, but its a trade off for ensuring all possible paths are considered. I am thinking of keeping a visited nodes list, but unsure if that will cause conflict
There was a problem hiding this comment.
I am not sure about this, this article is talking about causal effects. But here we are only testing conditional independence. These are related but not entirely identical concepts.
Also I think this is a good example why it is important to communicate why you implement the algorithms in the way you do.
There was a problem hiding this comment.
Ah i see makes sense
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…inition (#237) The motivation for the PR is initially to allow the use [`product_distribution`](https://juliastats.org/Distributions.jl/stable/multivariate/#Distributions.product_distribution) in JuliaBUGS model. By writing ```julia @bugs begin x[1:2] ~ Distributions.product_distribution(fill(Normal(), 2)) end ``` It also enables ```julia julia> foo(x) = x + 1 foo (generic function with 1 method) julia> model_def = @bugs begin a = Main.foo(b) end quote a = Main.foo(b) end julia> compile(model_def, (;b=2)) BUGSModel (parameters are in transformed (unconstrained) space, with dimension 0): Model parameters: Variable sizes and types: a: type = Int64 b: type = Int64 ``` i.e., an easier way to introduce external functions into JuliaBUGS. This can't fully replace `@register_primitive` yet, because using `Main` module is relying on Julia runtime behavior and not very intuitive. Examples: ```julia julia> module TestModule bar(x) = x + 1 end Main.TestModule julia> model_def = @bugs begin a = TestModule.bar(b) end quote a = TestModule.bar(b) end julia> compile(model_def, (; b = 1)) ERROR: UndefVarError: `TestModule` not defined in `JuliaBUGS` ... ``` one would need to do ```julia julia> model_def = @bugs begin a = Main.TestModule.bar(b) end quote a = Main.TestModule.bar(b) end ``` also ```julia julia> @testset "t" begin foo1(x) = x + 1 model_def = @bugs begin a = Main.foo1(b) end compile(model_def, (; b= 1)) end t: Error During Test at REPL[18]:1 Got exception outside of a @test UndefVarError: `foo1` not defined in `Main` ... ```
|
@naseweisssss thanks a lot for the effort, congrats for the first PR! |
No description provided.