Refactor graph and model part of the code #80

sunxd3 · 2023-09-03T10:01:45Z

This PR is aim to address the following points:

Crude interfaces for NodeInfo, Model, and Context
- NodeInfo
  - eval function for NodeInfo should hide the field of NodeInfo, this allows storing a Function instead of just Expr in the NodeInfo by implementing a eval function for new NodeInfo subtypes
  - a Module can be passed to eval, which will allow more freedom in the evaluation, this is not yet exposed
- Model
  - some interface functions like get_graph, transformation, node_iterator are implemented so that future model can share default implementation of evaluate!!
- Context
  - observe, assume, and logical_evaluate are implemented; given an order defined by node_iterator, one of these functions will be called on the nodes
  - LogDensityContext's evaluate!! is still implemented separately, maybe we can unify the interface further in the future
Fix compatibility with trans-dimensional bijectors
- When initializing the Model, two param_lengths are computed, one for without transformation, the other will consider the size of the transformed distribution
Link function in stochastic assignment is (temporarily) disabled
- This feature was not well tested before
- This is a feature supported only by nimble, not by the original BUGS
- There are some implementation complexity mainly from the fact that link function are essentially used specified bijectors, but since we have random variable transformation mechanism already (via Bijectors), this is not essential
Model definitions are separated into another file
Tests are also partly refactored
- We do not rely on change behavior of DynamicPPL anymore, instead, now we create a VarInfo whose values are transformed
- Also use the LogDensityFunction interface at points

sunxd3 · 2023-09-03T10:07:31Z

@yebai apologize for another PR with multiple changes, I hope it's not too confusing.

codecov · 2023-09-03T10:10:38Z

Codecov Report

Patch coverage: 88.69% and project coverage change: +5.28% 🎉

Comparison is base (ec81b3e) 69.20% compared to head (568c397) 74.49%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #80      +/-   ##
==========================================
+ Coverage   69.20%   74.49%   +5.28%     
==========================================
  Files          17       19       +2     
  Lines        1640     1635       -5     
==========================================
+ Hits         1135     1218      +83     
+ Misses        505      417      -88

Files Changed	Coverage Δ
src/JuliaBUGS.jl	`94.87% <ø> (ø)`
src/compiler_pass.jl	`90.90% <ø> (ø)`
src/parser.jl	`92.69% <ø> (ø)`
src/node_functions.jl	`91.90% <66.66%> (-0.50%)`	⬇️
src/model.jl	`84.97% <84.97%> (ø)`
src/utils.jl	`95.12% <95.12%> (ø)`
ext/JuliaBUGSAdvancedHMCExt.jl	`100.00% <100.00%> (+100.00%)`	⬆️
src/graphs.jl	`88.48% <100.00%> (+17.31%)`	⬆️
src/logdensityproblems.jl	`71.42% <100.00%> (+71.42%)`	⬆️
src/variable_types.jl	`78.57% <100.00%> (+2.57%)`	⬆️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/model.jl

src/graphs.jl

src/model.jl

test/logp_tests/lkj.jl

test/logp_tests/rats.jl

yebai

This is a very big PR; I reviewed half of it. I'll try to complete it tomorrow.

yebai · 2023-09-04T16:01:36Z

Project.toml

@@ -1,6 +1,6 @@
 name = "JuliaBUGS"
 uuid = "ba9fb4c0-828e-4473-b6a1-cd2560fee5bf"
-version = "0.1.0"
+version = "0.1.1"


Suggested change

version = "0.1.1"

version = "0.2"

yebai · 2023-09-04T16:03:16Z

README.md

@@ -197,7 +201,7 @@ n_samples, n_adapts = 2000, 1000

 D = LogDensityProblems.dimension(model); initial_θ = rand(D)

-samples_and_stats = AbstractMCMC.sample(
+@run samples_and_stats = AbstractMCMC.sample(


Suggested change

@run samples_and_stats = AbstractMCMC.sample(

samples_and_stats = AbstractMCMC.sample(

yebai · 2023-09-04T16:05:58Z

docs/src/program_evaluation.md

@@ -0,0 +1,10 @@
+# Evaluation of `BUGSModel`  
+
+During model evaluation, `varinfo` serves as the runtime environment, housing the current values of variables.


Suggested change

During model evaluation, `varinfo` serves as the runtime environment, housing the current values of variables.

During model evaluation, `varinfo` stores the current variables' values.

yebai · 2023-09-04T16:10:19Z

docs/src/program_evaluation.md

+# Evaluation of `BUGSModel`  
+
+During model evaluation, `varinfo` serves as the runtime environment, housing the current values of variables.
+These values are always stored in their native (possibly constraint) spaces.


Suggested change

These values are always stored in their native (possibly constraint) spaces.

These variables are always stored as-is, i.e. without transformation.

yebai · 2023-09-04T16:11:24Z

docs/src/program_evaluation.md

+
+During model evaluation, `varinfo` serves as the runtime environment, housing the current values of variables.
+These values are always stored in their native (possibly constraint) spaces.
+The `varinfo` represents the state of program evaluation at different evaluation points, given a chosen order of node evaluation.


Suggested change

The `varinfo` represents the state of program evaluation at different evaluation points, given a chosen order of node evaluation.

The `varinfo` represents the state of program evaluation at different evaluation points, given a chosen ordering of node evaluations.

yebai · 2023-09-04T16:24:34Z

src/JuliaBUGS.jl

+@enum VariableTypes::Bool begin
+    Logical
+    Stochastic
+end
+
+@enum StochasticVariableTypes::Bool begin
+    Observation
+    Assumption
+end
+


Maybe merge these two types:

@enum VariableTypes Logical Observation Assumption

yebai · 2023-09-04T16:27:11Z

src/graphs.jl

+"""
+    eval([m::Module, ]ni::ConcreteNodeInfo, vi)
+
+Evaluate a node under a specified module `m`. If no module is provided, the default module used is JuliaBUGS.


Evaluating nodes in JuliaBUGS sounds terrible; why not default to Main?

yebai · 2023-09-04T16:31:13Z

src/graphs.jl

-
-"""
-    LogDensityContext
+Find the Markov blanket of `v` in `g`. `v` can be a single `VarName` or a vector of `VarName`.


Suggested change

Find the Markov blanket of `v` in `g`. `v` can be a single `VarName` or a vector of `VarName`.

Find the Markov blanket of variable(s) `v` in graph `g`. `v` can be a single `VarName` or a vector of `VarName`.

yebai · 2023-09-04T16:31:47Z

src/graphs.jl

-        else
-            logp += logpdf(dist, value)
-        end
+function markov_blanket(g, v::VarName)


Suggested change

function markov_blanket(g, v::VarName)

function markov_blanket(g::BUGSGraph, v::VarName)

yebai · 2023-09-04T16:33:04Z

src/graphs.jl

-                logp += logpdf(dist, vi[vn])
-            end
-        end
+function markov_blanket(g, v)


Suggested change

function markov_blanket(g, v)

function markov_blanket(g::BUGSGraph,, v::Vector{VarName})

yebai · 2023-09-04T16:39:36Z

Please add some tests #66 and #71

sunxd3 · 2023-09-05T06:48:12Z

Thanks @yebai for the reviews, given this size of the PR and difficulty of fully reviewing, I will break it into a series of smaller PRs.
The review that are already proposed will be reflected.

sunxd3 · 2023-09-17T11:55:18Z

This PR has been broken down into #86, #88, #89, #90

sunxd3 added 13 commits August 30, 2023 15:46

Move _eval to utils.jl

6b80455

Update evaluate!! function

3f97bcd

Add primitive interfaces, reduce repeated code

c0f9362

Refactor logp computation for LogDensityContext

786bbdd

Temp test change

559370f

Merge remote-tracking branch 'origin/master' into sunxd-refactor-eval

c4262c6

Bug fix; add test for lkj model

e16fa50

Fix logp computation, work with lkj

ba40c43

Separate model definition from graphs.jl

49e650e

Fix some test errors

cd14b79

Fix logp tests

1604913

Document varinfo during model evaluation

971e966

Formatting

0cdb5d8

sunxd3 requested a review from yebai September 3, 2023 10:06

sunxd3 marked this pull request as draft September 3, 2023 10:38

Set to DynamicTransformation by default

230feb0

github-actions bot reviewed Sep 3, 2023

View reviewed changes

src/model.jl Outdated Show resolved Hide resolved

sunxd3 added 3 commits September 3, 2023 14:04

Update Project.toml

cb0ef0f

Fix logp value when transformed

20ca5d8

Fix tests.

ca0c00f

github-actions bot reviewed Sep 4, 2023

View reviewed changes

Formatting

f92e6fb

github-actions bot reviewed Sep 4, 2023

View reviewed changes

test/logp_tests/rats.jl Outdated Show resolved Hide resolved

sunxd3 added 3 commits September 4, 2023 09:10

Format and fix Project.toml

b81421e

Fix Project.toml

60198a8

Fix test error

af63b99

sunxd3 marked this pull request as ready for review September 4, 2023 08:31

sunxd3 mentioned this pull request Sep 4, 2023

Models with dwish and ddirich are not yet supported with AdvancedHMC #66

Closed

sunxd3 added 2 commits September 4, 2023 13:46

Store the sizes of each parameters in model

a2739ce

Handle multi-dim varname HMCExt module

568c397

yebai reviewed Sep 4, 2023

View reviewed changes

sunxd3 marked this pull request as draft September 5, 2023 06:48

sunxd3 closed this Sep 17, 2023

yebai deleted the sunxd-refactor-eval branch September 17, 2023 15:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor graph and model part of the code #80

Refactor graph and model part of the code #80

sunxd3 commented Sep 3, 2023 •

edited

Loading

sunxd3 commented Sep 3, 2023

codecov bot commented Sep 3, 2023 •

edited

Loading

yebai left a comment

yebai Sep 4, 2023

yebai Sep 4, 2023

yebai Sep 4, 2023

yebai Sep 4, 2023

yebai Sep 4, 2023

yebai Sep 4, 2023

yebai Sep 4, 2023

yebai Sep 4, 2023

yebai Sep 4, 2023

yebai Sep 4, 2023

yebai commented Sep 4, 2023

sunxd3 commented Sep 5, 2023

sunxd3 commented Sep 17, 2023

	@run samples_and_stats = AbstractMCMC.sample(
	samples_and_stats = AbstractMCMC.sample(

		@@ -0,0 +1,10 @@
		# Evaluation of `BUGSModel`

		During model evaluation, `varinfo` serves as the runtime environment, housing the current values of variables.

	During model evaluation, `varinfo` serves as the runtime environment, housing the current values of variables.
	During model evaluation, `varinfo` stores the current variables' values.

	These values are always stored in their native (possibly constraint) spaces.
	These variables are always stored as-is, i.e. without transformation.

	The `varinfo` represents the state of program evaluation at different evaluation points, given a chosen order of node evaluation.
	The `varinfo` represents the state of program evaluation at different evaluation points, given a chosen ordering of node evaluations.

	Find the Markov blanket of `v` in `g`. `v` can be a single `VarName` or a vector of `VarName`.
	Find the Markov blanket of variable(s) `v` in graph `g`. `v` can be a single `VarName` or a vector of `VarName`.

	function markov_blanket(g, v::VarName)
	function markov_blanket(g::BUGSGraph, v::VarName)

	function markov_blanket(g, v)
	function markov_blanket(g::BUGSGraph,, v::Vector{VarName})

Refactor graph and model part of the code #80

Refactor graph and model part of the code #80

Conversation

sunxd3 commented Sep 3, 2023 • edited Loading

sunxd3 commented Sep 3, 2023

codecov bot commented Sep 3, 2023 • edited Loading

Codecov Report

yebai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yebai commented Sep 4, 2023

sunxd3 commented Sep 5, 2023

sunxd3 commented Sep 17, 2023

sunxd3 commented Sep 3, 2023 •

edited

Loading

codecov bot commented Sep 3, 2023 •

edited

Loading