-
Notifications
You must be signed in to change notification settings - Fork 31
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.
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
Rework of how transformations are handled #575
Merged
Merged
Changes from 169 commits
Commits
Show all changes
198 commits
Select commit
Hold shift + click to select a range
5af1afa
initial implementation of VarNameVector
torfjelde 8ce53f7
added some hacky getval and getdist get things to work for VarInfo
torfjelde fc6a051
Apply suggestions from code review
torfjelde 7cd599d
added arbitrary metadata field as discussed
torfjelde ed0a757
renamed idcs to varname_to_index
torfjelde 4ebd252
renamed vns to varnames for VarNameVector
torfjelde 9f12c9a
added keys impl for Metadata
torfjelde 5a15121
added push! and update! for VarNameVector
torfjelde edde2c1
added getindex_raw! and setindex_raw! for VarNameVector
torfjelde ed46002
added `iterate` and `convert` (for `AbstractDict) impls for `VarNameV…
torfjelde 5b00059
make the key and eltype part of the `VarNameVector` type
torfjelde bef7e0a
added more tests for VarNameVector
torfjelde 006ee8d
formatting
torfjelde 9802811
more testing for VarNameVector
torfjelde 88b1721
minor changes to some comments
torfjelde ca7b173
added a bunch more tests for VarNameVector + several bugfixes in the …
torfjelde fb01b94
formatting
torfjelde 9634839
added `similar` implementation for `VarNameVector`
torfjelde 5179f6f
formatting
torfjelde 9f632bb
removed debug statement
torfjelde 3c210f7
made VarInfo slighly more generic wrt. underlying metadata
torfjelde 8bf6589
Merge branch 'master' into torfjelde/varnamevector
torfjelde 8b2720f
fixed incorrect behavior in `keys` for `Metadata`
torfjelde 9fa6446
minor style changes to VarNameVector tests
torfjelde 0900c57
style
torfjelde 1f7e633
added testing of `update!` with smaller sizes and fixed bug related t…
torfjelde 8d05586
formatting
torfjelde 7801fe1
move functionality related to `push!` for `VarNameVector` into `push!`
torfjelde cdc2373
Update src/varnamevector.jl
torfjelde d2d776d
Merge branch 'master' into torfjelde/varnamevector
torfjelde ae4bcb7
several fixes to make sampling with VarNameVector + initiall tests for
torfjelde 97e1bcc
VarInfo + VarNameVector tests for all demo models
torfjelde be3c1b4
Merge remote-tracking branch 'origin/torfjelde/varnamevector' into to…
torfjelde ad343f3
Apply suggestions from code review
torfjelde f707b25
added docs on the design of `VarNameVector`
torfjelde 4e7af1d
Merge branch 'master' into torfjelde/varnamevector
torfjelde f1faf18
Apply suggestions from code review
torfjelde 87d3d01
added note on `update!`
torfjelde 9c3b265
further elaboration of the design of `VarInfo` and `VarNameVector`
torfjelde 958c66b
more writing improvements
torfjelde 74c6efd
added docstring to `has_inactive_ranges` and `inactive_ranges_sweep!`
torfjelde d9ea878
moved docs on `VarInfo` design to a separate internals section
torfjelde 5acce98
writing improvements for internal docs
torfjelde 6f95cdd
further motivation of the design choices made in `VarNameVector`
torfjelde 38a4b08
improved writing
torfjelde 60edd10
VarNameVector is now grown as much as needed
torfjelde 3f9d34f
updated `delete!`
torfjelde fb822b5
Significant changes to implementation of `VarNameVector`:
torfjelde 66bc090
added `copy` when constructing `VectorVarInfo` from `VarInfo`
torfjelde ccd86f2
added missing `isempty` impl
torfjelde 1d4a000
remove impl of `iterate` and instead implemented `pairs` and `values`…
torfjelde 9a16dd1
added missing `empty!` for `num_inactive`
torfjelde e49b762
removed redundant `shift_left!` methd
torfjelde 2b445c9
fixed `delete!` for `VarNameVector`
torfjelde e3c2633
added `is_contiguous` as an alterantive to `!has_inactive`
torfjelde 19a829c
updates to internal docs
torfjelde a358bc4
renamed `sweep_inactive_ranges!` to `contiguify!`
torfjelde 46be8d5
improvements to internal docs
torfjelde 57d688e
more improvements to internal docs
torfjelde 0968a07
moved additional methods description in internals to earlier in the doc
torfjelde 0d008a4
moved internals docs to a separate directory and split into files
torfjelde ccd0d64
more improvements to internals doc
torfjelde 7c45e67
formatting
torfjelde 373215b
added tests for `delete!` and fixed reference to old method
torfjelde 0cdafbf
addition to `delete!` test
torfjelde 51c041f
added `values_as` impls for `VarNameVector`
torfjelde 20b3742
added docs for `replace_valus` and `values_as` for `VarNameVector`
torfjelde ef6c618
fixed doctest
torfjelde 8a1209c
formatting
torfjelde adeadf0
temporarily disable doctests so we can build docs
torfjelde 7ff179d
added missing compat entry for ForwardDiff in docs
torfjelde c7ec08a
moved some shared code into methods to make things a bit cleaner
torfjelde c5a5e58
added impl of `merge` for `VarNameVector`
torfjelde c376d95
renamed a few variables in `merge` impl for `VarNameVector`
torfjelde f71baa5
forgot to include some changes in previous commit
torfjelde af25f3c
added impl of `subset` for `VarNameVector`
torfjelde c28f076
fixed `pairs` impl for `VarNameVector`
torfjelde f5d2c63
added missing impl of `subset` for `VectorVarInfo`
torfjelde 3eb6c7f
added missing impl of `merge_metadata` for `VarNameVector`
torfjelde 9ba8144
added a bunch of `from_vec_transform` and `tovec` impls to make
torfjelde acd6951
make default args use `from_vec_transform` rather than `FromVec`
torfjelde 790f743
fixed `values_as` fro `VarInfo` with `VarNameVector` as `metadata`
torfjelde c474bb0
fixed impl of `getindex_raw` when using integer index for `VarNameVec…
torfjelde 8251463
added tests for `getindex` with `Int` index for `VarNameVector`
torfjelde 5df7031
fix for `setindex!` and `setindex_raw!` for `VarNameVector`
torfjelde 683b776
introduction of `from_vec_transform` and `tovec` and its usage in `Va…
torfjelde 4dae00d
moved definition of `is_splat_symbol` to the file where it's used
torfjelde e3b52a4
added `VarInfo` constructor with vector input for `VectorVarInfo`
torfjelde 9626be1
make `extract_priors` take the `rng` as an argument
torfjelde e731fd6
added `replace_values` for `Metadata`
torfjelde 0785abf
make link and invlink act on the `metadata` field for `VarInfo` +
torfjelde b3e0955
added temporary defs of `with_logabsdet_jacobian` and `inverse` for
torfjelde ff963ce
added invlink_with_logpdf overload for `ThreadSafeVarInfo`
torfjelde 03f2b2b
added `is_transformed` field to `VarNameVector`
torfjelde 949b33a
removed unnecessary defintions of `with_logabsdet_jacobian` and
torfjelde cc5ecc4
fixed issue where we were storing the wrong transformations in `VarNa…
torfjelde 1aae1b4
make sure `extract_priors` doesn't mutate the `varinfo`
torfjelde 8e0853d
updated `similar` for `VarNameVector` and fixed `invlink` for `VarNam…
torfjelde 229b168
added handling of `is_transformed` in `merge` for `VarNameVector`
torfjelde c581dcf
removed unnecesasry `deepcopy` from outer `link`
torfjelde b4d3f55
updated `push!` to also `push!` on `is_transformed`
torfjelde ed1d006
skip tests for mutating linking when using VarNameVector
torfjelde f132209
use same projection for `Cholesky` in `VarNameVector` as in `VarInfo`
torfjelde 49454de
fixed `settrans!!` for `VarInfo` with `VarNameVector`
torfjelde 01ff2dd
fixed bug in `set_flag!`
torfjelde 20adedf
fixed another typo
torfjelde 8f9566a
fixed return values of `settrans!!`
torfjelde 5532046
updated static transformation tests
torfjelde 3c5d2ac
Update test/simple_varinfo.jl
torfjelde 317d969
Merge branch 'master' into torfjelde/varnamevector
torfjelde f8441ea
Merge remote-tracking branch 'origin/torfjelde/varnamevector' into to…
torfjelde ab16323
Merge branch 'master' into torfjelde/varnamevector
torfjelde a9be219
removed unnecessary impl of `extract_priors`
torfjelde 53c8d33
make `short_varinfo_name` of `TypedVarInfo` a bit more informative
torfjelde 61d85ad
moved impl of `has_varnamevector` for `ThreadSafeVarInfo`
torfjelde 9ace554
added back `extract_priors` impl as we do need it
torfjelde 67c9821
forgot to include tests for `VarNameVector` in `runtests.jl`
torfjelde 32a2d31
fix for `relax_container_types` in `test/varnamevector.jl`
torfjelde b3bb42d
fixed `need_transforms_relaxation`
torfjelde 25ff2b1
updated some tests to not refer directly to `FromVec`
torfjelde 004f038
introduce `from_internal_transform` and its siblings
torfjelde 38c89bd
remove `with_logabsdet_jacobian_and_reconstruct` in favour of
torfjelde 218dc23
added `internal_to_linked_internal_transform` + fixed a few bugs in
torfjelde 1df4293
added `linked_internal_to_internal_transform` as a complement to `int…
torfjelde f8df896
fixed bugs in `invlink` for `VarInfo` using `linked_internal_to_inter…
torfjelde d62f26a
more work on removing calls to `reconstruct`
torfjelde b4517d6
removed redundant comment
torfjelde b7d4754
added `from_linked_vec_transform` specialization for `LKJ`
torfjelde 0244dd9
more work on removing references to `reconstruct`
torfjelde e886d07
added `copy` in `values_from_metadata` to preserve behavior and avoid
torfjelde 2af6605
remove `reconstruct_and_link` and `invlink_and_reconstruct`
torfjelde a0664d7
replaced references to `link_and_reconstruct` and `invlink_and_recons…
torfjelde f2d59b2
introduced `recombine` and replaced calls to `reconstruct` with `n` s…
torfjelde e3bfa76
completely removed `reconstruct`
torfjelde c0aef81
renamed `maybe_reconstruct_and_link` to `to_maybe_linked_internal` and
torfjelde f7c0853
added impls of `from_*_internal_transform` for `ThreadSafeVarInfo`
torfjelde 77b835e
removed `reconstruct` from docs and from exports
torfjelde b83c262
renamed `getval` to `getindex_internal` and made `dist` an optional
torfjelde c4faf3e
updated docs + added description of how internals of transforms work
torfjelde c8d9695
added a bunch of illustrations for the transforms docs + dot files us…
torfjelde 95dc8e3
temporarily removed `VarNameVector` completely
torfjelde 8930f9c
formatting
torfjelde e45b668
Update docs/src/internals/transformations.md
torfjelde 0e71092
Update docs/src/internals/transformations.md
torfjelde 2de9ac9
removed refs to VectorVarInfo
torfjelde 9b71428
added impls of `from_internal_transform` for `ThreadSafeVarInfo`
torfjelde 786e9bf
reverted accidental removal of old `VarInfo` constructor
torfjelde f1fe42c
fixed incorrect `recombine` call
torfjelde 2273954
removed undefined refs to `VarNameVector` stuff in `setup_varinfos`
torfjelde ab7c189
bump minior version because Turing breaks
torfjelde 3a86601
fix: was using `from_linked_internal_transform` in
torfjelde 28c7d85
removed `getindex_raw`
torfjelde 59514d6
removed redundant docstrings
torfjelde cdc882b
fixed tests
torfjelde 57ba7c0
fixed comparisons in tests
torfjelde 902e59c
try relative references for images in transformation docs
torfjelde d7aba55
another attempt at fixing asset-references
torfjelde b0dd2f8
Merge branch 'master' into torfjelde/transformations
torfjelde 1f51203
fixed getindex diagrams in docs
torfjelde 0eb79b1
minor changes to comments
torfjelde 071bebf
remove Combinatorics as a test dep, as it's not needed for this PR
torfjelde bbdc060
reverted unnecessary change
torfjelde e2f4d18
disable type-stability tests for models on older Julia versions
torfjelde 3d823ac
removed seemingly completely unused impl of `setval!`
torfjelde 8750255
Merge branch 'master' into torfjelde/transformations
torfjelde ed6ee88
Merge branch 'master' into torfjelde/transformations
torfjelde 607bdb3
Update test/model.jl
torfjelde 55c8098
Apply suggestions from code review
torfjelde cc910d5
Merge remote-tracking branch 'origin/torfjelde/transformations' into …
torfjelde ec9f985
Merge branch 'master' into torfjelde/transformations
torfjelde a079606
Merge remote-tracking branch 'origin/torfjelde/transformations' into …
torfjelde ad959ec
Type-stability tests are now correctly using `rand_prior_true` instead
torfjelde 9f84070
`getindex_internal` now calls `getindex` instead of `view`, as the
torfjelde 7d39934
Removed seemingly unnecessary definition of `getindex_internal`
torfjelde b554504
Fixed references to `newmetadata` which has been replaced by `replace…
torfjelde ddb1dfe
Made implementation of `recombine` more explicit
torfjelde 3b08f1d
Added docstrings for `untyped_varinfo` and `typed_varinfo`
torfjelde 96ccebe
Added TODO comment about implementing `view` for `VarInfo`
torfjelde beaeeaa
Fixed potential infinite recursion as suggested by @mhauru
torfjelde ab2c98b
added docstring to `from_vec_trnasform_for_size
torfjelde f1f7968
Replaced references to `vectorize(dist, x)` with `tovec(x)`
torfjelde 6e57822
Fixed docstring
torfjelde 841215f
Update src/extract_priors.jl
torfjelde 78b2083
Bump minor version since this is a breaking change
torfjelde b6ecf7b
Merge remote-tracking branch 'origin/torfjelde/transformations' into …
torfjelde 7100ce1
Merge branch 'master' into torfjelde/transformations
torfjelde bab63e1
Apply suggestions from code review
sunxd3 6997019
Update src/varinfo.jl
sunxd3 9dc7f02
Apply suggestions from code review
torfjelde c0f9923
Apply suggestions from code review
torfjelde 9056928
Update src/extract_priors.jl
torfjelde e43dd1b
Added fix for product distributions of targets with changing support …
torfjelde a7673fd
Addeed tests for product of distributions with dynamic support
torfjelde e8d4c96
Apply suggestions from code review
torfjelde d7c224e
Empty commit to trigger CI
mhauru 951ffd5
Merge remote-tracking branch 'origin/master' into torfjelde/transform…
mhauru a0a8761
Update test/model.jl
torfjelde a8812e9
Increase HTML page size threshold for docs
mhauru File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
17 changes: 17 additions & 0 deletions
17
docs/src/assets/images/transformations-assume-without-istrans.dot
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
digraph { | ||
# `assume` block | ||
subgraph cluster_assume { | ||
label = "assume"; | ||
fontname = "Courier"; | ||
|
||
assume [shape=box, label=< assume(varinfo, <FONT COLOR="#3B6EA8">@varname</FONT>(x), Normal())>, fontname="Courier"]; | ||
without_linking_assume [shape=box, label="f = from_internal_transform(varinfo, varname, dist)", fontname="Courier"]; | ||
with_logabsdetjac [shape=box, label="x, logjac = with_logabsdet_jacobian(f, assume_internal(varinfo, varname, dist))", fontname="Courier"]; | ||
return_assume [shape=box, label=< <FONT COLOR="#3B6EA8">return</FONT> x, logpdf(dist, x) - logjac, varinfo >, style=dashed, fontname="Courier"]; | ||
|
||
assume -> without_linking_assume; | ||
without_linking_assume -> with_logabsdetjac; | ||
with_logabsdetjac -> return_assume; | ||
} | ||
} | ||
|
Binary file added
BIN
+25.3 KB
docs/src/assets/images/transformations-assume-without-istrans.dot.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
88 changes: 88 additions & 0 deletions
88
docs/src/assets/images/transformations-assume-without-istrans.dot.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
digraph { | ||
# `assume` block | ||
subgraph cluster_assume { | ||
label = "assume"; | ||
fontname = "Courier"; | ||
|
||
assume [shape=box, label=< assume(varinfo, <FONT COLOR="#3B6EA8">@varname</FONT>(x), Normal())>, fontname="Courier"]; | ||
iflinked_assume [label=< <FONT COLOR="#3B6EA8">if</FONT> istrans(varinfo, varname) >, fontname="Courier"]; | ||
without_linking_assume [shape=box, label="f = from_internal_transform(varinfo, varname, dist)", fontname="Courier"]; | ||
with_linking_assume [shape=box, label="f = from_linked_internal_transform(varinfo, varname, dist)", fontname="Courier"]; | ||
with_logabsdetjac [shape=box, label="x, logjac = with_logabsdet_jacobian(f, assume_internal(varinfo, varname, dist))", fontname="Courier"]; | ||
return_assume [shape=box, label=< <FONT COLOR="#3B6EA8">return</FONT> x, logpdf(dist, x) - logjac, varinfo >, style=dashed, fontname="Courier"]; | ||
|
||
assume -> iflinked_assume; | ||
iflinked_assume -> without_linking_assume [label=< <FONT COLOR="#97365B">false</FONT>>, fontname="Courier"]; | ||
iflinked_assume -> with_linking_assume [label=< <FONT COLOR="#97365B">true</FONT>>, fontname="Courier"]; | ||
without_linking_assume -> with_logabsdetjac; | ||
with_linking_assume -> with_logabsdetjac; | ||
with_logabsdetjac -> return_assume; | ||
} | ||
} | ||
|
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
20 changes: 20 additions & 0 deletions
20
docs/src/assets/images/transformations-getindex-with-dist.dot
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
digraph { | ||
# `getindex` block | ||
subgraph cluster_getindex { | ||
label = "getindex"; | ||
fontname = "Courier"; | ||
|
||
getindex [shape=box, label=< x = getindex(varinfo, <FONT COLOR="#3B6EA8">@varname</FONT>(x), Normal()) >, fontname="Courier"]; | ||
iflinked_getindex [label=< <FONT COLOR="#3B6EA8">if</FONT> istrans(varinfo, varname) >, fontname="Courier"]; | ||
without_linking_getindex [shape=box, label="f = from_internal_transform(varinfo, varname, dist)", fontname="Courier"]; | ||
with_linking_getindex [shape=box, label="f = from_linked_internal_transform(varinfo, varname, dist)", fontname="Courier"]; | ||
return_getindex [shape=box, label=< <FONT COLOR="#3B6EA8">return</FONT> f(getindex_internal(varinfo, varname)) >, style=dashed, fontname="Courier"]; | ||
|
||
getindex -> iflinked_getindex; | ||
iflinked_getindex -> without_linking_getindex [label=< <FONT COLOR="#97365B">false</FONT>>, fontname="Courier"]; | ||
iflinked_getindex -> with_linking_getindex [label=< <FONT COLOR="#97365B">true</FONT>>, fontname="Courier"]; | ||
without_linking_getindex -> return_getindex; | ||
with_linking_getindex -> return_getindex; | ||
} | ||
} | ||
|
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
20 changes: 20 additions & 0 deletions
20
docs/src/assets/images/transformations-getindex-without-dist.dot
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
digraph { | ||
# `getindex` block | ||
subgraph cluster_getindex { | ||
label = "getindex"; | ||
fontname = "Courier"; | ||
|
||
getindex [shape=box, label=< x = getindex(varinfo, <FONT COLOR="#3B6EA8">@varname</FONT>(x)) >, fontname="Courier"]; | ||
iflinked_getindex [label=< <FONT COLOR="#3B6EA8">if</FONT> istrans(varinfo, varname) >, fontname="Courier"]; | ||
without_linking_getindex [shape=box, label="f = from_internal_transform(varinfo, varname)", fontname="Courier"]; | ||
with_linking_getindex [shape=box, label="f = from_linked_internal_transform(varinfo, varname)", fontname="Courier"]; | ||
return_getindex [shape=box, label=< <FONT COLOR="#3B6EA8">return</FONT> f(getindex_internal(varinfo, varname)) >, style=dashed, fontname="Courier"]; | ||
|
||
getindex -> iflinked_getindex; | ||
iflinked_getindex -> without_linking_getindex [label=< <FONT COLOR="#97365B">false</FONT>>, fontname="Courier"]; | ||
iflinked_getindex -> with_linking_getindex [label=< <FONT COLOR="#97365B">true</FONT>>, fontname="Courier"]; | ||
without_linking_getindex -> return_getindex; | ||
with_linking_getindex -> return_getindex; | ||
} | ||
} | ||
|
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
digraph { | ||
# Nodes. | ||
tilde_node [shape=box, label="x ~ Normal()", fontname="Courier"]; | ||
base_node [shape=box, label=< vn = <FONT COLOR="#3B6EA8">@varname</FONT>(x)<BR/>dist = Normal()<BR/>x, vi = ... >, fontname="Courier"]; | ||
assume [shape=box, label="assume(vn, dist, vi)", fontname="Courier"]; | ||
|
||
iflinked [label=< <FONT COLOR="#3B6EA8">if</FONT> istrans(vi, vn) >, fontname="Courier"]; | ||
|
||
without_linking [shape=box, label="f = from_internal_transform(vi, vn, dist)", styled=dashed, fontname="Courier"]; | ||
with_linking [shape=box, label="f = from_linked_internal_transform(vi, vn, dist)", styled=dashed, fontname="Courier"]; | ||
|
||
with_logabsdetjac [shape=box, label="x, logjac = with_logabsdet_jacobian(f, getindex_internal(vi, vn, dist))", styled=dashed, fontname="Courier"]; | ||
return [shape=box, label=< <FONT COLOR="#3B6EA8">return</FONT> x, logpdf(dist, x) - logjac, vi >, styled=dashed, fontname="Courier"]; | ||
|
||
# Edges. | ||
tilde_node -> base_node [style=dashed, label=< <FONT COLOR="#3B6EA8">@model</FONT>>, fontname="Courier"] | ||
base_node -> assume [style=dashed, label=" tilde-pipeline", fontname="Courier"]; | ||
|
||
assume -> iflinked; | ||
|
||
iflinked -> without_linking [label=< <FONT COLOR="#97365B">false</FONT>>, fontname="Courier"]; | ||
iflinked -> with_linking [label=< <FONT COLOR="#97365B">true</FONT>>, fontname="Courier"]; | ||
|
||
without_linking -> with_logabsdetjac; | ||
with_linking -> with_logabsdetjac; | ||
|
||
with_logabsdetjac -> return; | ||
} |
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Random thought: Could we use DocumenterMermaid and write the graphs as mermaid code blocks in the docs (BTW mermaid is supported also by Github: https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/creating-diagrams)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be happy with this 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I'm having a look at this, and the main issue is that we can't do proper syntax highlighting 😕 This is a bit annoying, but also agree that the PNGs needed for the DOT diagrams are also annoying.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, so after some hacking, I think I have a solution that gets us syntax highlighting and stuff in Mermaid.js diagrams, with the caveat that we can't get proper colouring.
Mermaid.js uses some pretty strong CSS rules, e.g. unique ID for every diagram. This in turn means that it's difficult to override generally (you have to do it case-by-case for each diagram), and even then, you can't completely make use of the highlight.js functionality to get highlighing in nodes because of the ID-based styling of Mermaid.js.
Buuut this should be sufficient to replace usage of dot diagrams
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mermaid-js/mermaid#5466
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shravanngoswamii can you help replace all figures in the PR with DocumenterMermaid? Feel free to open a new PR against this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@torfjelde It might be better to transfer these high-level docs into a Quarto docs page and then provide a link here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, Sure!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working on it: https://github.com/shravanngoswamii/DynamicPPL.jl/blob/1050a4860ea6f1a6ccbfff8184e60b6e7e79478b/docs/src/internals/transformations.md?plain=1#L223C1-L246C4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly! But I think that's somewhat out of scope for this PR? This is following the existing approach in DPPL of documenting things here, so if we want to move these things, we should do so once and proper I think