Graphviz suggestions

[bvdmitri]

This pull request introduces several improvements and new functionalities to the GraphPPLGraphVizExt.jl file, enhancing the visualization capabilities and code maintainability. The most important changes include importing the load function from GraphViz, adding new helper functions for label display, updating node addition logic, and modifying the generate_dot function to include additional options.

After this change

Enhancements to visualization:

• ext/GraphPPLGraphVizExt.jl: Imported the load function from GraphViz to enable loading and processing of graph models.
• ext/GraphPPLGraphVizExt.jl: Added get_displayed_label functions for FactorNodeProperties and VariableNodeProperties to customize node labels in the DOT string.
• ext/GraphPPLGraphVizExt.jl: Updated the add_nodes! function to include the displayed label in the node attributes for better visualization.

Code refactoring and new options:

• ext/GraphPPLGraphVizExt.jl: Refactored the generate_dot function to use the GraphViz.load method, adding new parameters such as show and save_to for enhanced functionality.
• src/graph_engine.jl: Added a getname function for FactorNodeProperties to retrieve the name of the properties, improving code readability and reusability.

[bvdmitri]

Additionally a user can define their own pretty names, e.g.

GraphPPL.prettyname(::Type{<:NormalMeanPrecision}) = "N"
GraphPPL.prettyname(::Type{<:GammaShapeScale}) = "Γ"

results in

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 90.44%. Comparing base (e49dcb5) to head (14eb077).
Report is 5 commits behind head on graphviz_issue_233.

Files with missing lines	Patch %	Lines
src/graph_engine.jl	0.00%	3 Missing ⚠️

Additional details and impacted files

@@                  Coverage Diff                   @@
##           graphviz_issue_233     #257      +/-   ##
======================================================
- Coverage               90.61%   90.44%   -0.18%     
======================================================
  Files                      15       15              
  Lines                    2121     2124       +3     
======================================================
- Hits                     1922     1921       -1     
- Misses                    199      203       +4     

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

---

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests

[bvdmitri]

I didn't change tests, so I guess this might break things. But this is also the idea for @chbe-helix and @FraserP117 on how I would implement "pretty" naming

[FraserP117]

Thanks @bvdmitri!

Excellent stuff. I had actually implemented the GraphViz.load overload in the graphviz_issue_233_node_naming branch but we will roll with your specific version from now. It will break some of my tests but they need revision anyway.

The pretty naming is also great, I'll check that out.

Thanks, talk soon.

[FraserP117]

I have attempted to call this method in a version of the v2 test file that I wrote as follows:

push!(LOAD_PATH, joinpath(@__DIR__, "../ext"))

using GraphPPLGraphVizExt: GraphViz


## CREATE AN RXINFER.JL MODEL:
# GraphPPL.jl export `@model` macro for model specification
# It accepts a regular Julia function and builds an FFG under the hood
@model function coin_model(y, a, b)
    # We endow θ parameter of our model with some prior
    θ ~ Beta(a, b)
    # or, in this particular case, the `Uniform(0.0, 1.0)` prior also works:
    # θ ~ Uniform(0.0, 1.0)

    # We assume that outcome of each coin flip is governed by the Bernoulli distribution
    for i in eachindex(y)
        y[i] ~ Bernoulli(θ)
    end
end

# condition the model on some observed data
conditioned = coin_model(a = 2.0, b = 7.0) | (y = [ true, false, true ], )

# `Create` the actual graph of the model conditioned on the data
rxi_model = RxInfer.create_model(conditioned)

gppl_model = RxInfer.getmodel(rxi_model)


gen_dot_result_coin_simple = GraphViz.load(
    gppl_model,
    strategy = :simple,
    font_size = 12,
    edge_length = 1.0,
    layout = "neato",
    overlap = false,
    width = 10.0,
    height = 10.0,
    show = false
)

As yet, my use of GraphViz.load is resulting in a persistent failure to correctly dispatch on the overloaded method:

$ julia test/visualization_tests_233_v2.jl
ERROR: LoadError: MethodError: no method matching var"#load#3"(::Symbol, ::Int64, ::Float64, ::String, ::Bool, ::Float64, ::Float64, ::Bool, ::Nothing, ::typeof(GraphViz.load), ::GraphPPL.Model{MetaGraph{Int64, SimpleGraph{Int64}, GraphPPL.NodeLabel, GraphPPL.NodeData, GraphPPL.EdgeLabel, GraphPPL.Context, MetaGraphsNext.var"#4#8", Float64}, GraphPPL.PluginsCollection{Tuple{}}, RxInfer.ReactiveMPGraphPPLBackend{Static.False}})
The function `#load#3` exists, but no method is defined for this combination of argument types.

Closest candidates are:
  var"#load#3"(::Symbol, ::Int64, ::Float64, ::String, ::Bool, ::Float64, ::Float64, ::Bool, ::String, ::typeof(GraphViz.load), ::GraphPPL.Model)
   @ GraphPPLGraphVizExt ~/Desktop/GraphPPL.jl/ext/GraphPPLGraphVizExt.jl:637

Stacktrace:
 [1] load(model_graph::GraphPPL.Model{MetaGraph{Int64, SimpleGraph{Int64}, GraphPPL.NodeLabel, GraphPPL.NodeData, GraphPPL.EdgeLabel, GraphPPL.Context, MetaGraphsNext.var"#4#8", Float64}, GraphPPL.PluginsCollection{Tuple{}}, RxInfer.ReactiveMPGraphPPLBackend{Static.False}})
   @ GraphPPLGraphVizExt ~/Desktop/GraphPPL.jl/ext/GraphPPLGraphVizExt.jl:637
 [2] top-level scope
   @ ~/Desktop/GraphPPL.jl/test/visualization_tests_233_v2.jl:51

Perhaps I have just done something silly but I can't seem to shake this error, no matter how I re-define the positionality of the arguments.

[bvdmitri]

Update: Ok its my mistake, I pushed the fix, the type for the save_to variable was wrong.

I'm not sure about those lines

push!(LOAD_PATH, joinpath(@__DIR__, "../ext"))

using GraphPPLGraphVizExt: GraphViz

You should simply do

using GraphViz

and the package should be installed on your system. Otherwise the extension won't load properly. Does that fix the error?

You error however says something weird, so it definitely sees the method here in the stacktrace (I assume the extension loaded?)

load(model_graph::GraphPPL.Model{MetaGraph{Int64, SimpleGraph{Int64}, GraphPPL.NodeLabel, GraphPPL.NodeData, GraphPPL.EdgeLabel, GraphPPL.Context, MetaGraphsNext.var"#4#8", Float64}, GraphPPL.PluginsCollection{Tuple{}}, RxInfer.ReactiveMPGraphPPLBackend{Static.False}})
   @ GraphPPLGraphVizExt ~/Desktop/GraphPPL.jl/ext/GraphPPLGraphVizExt.jl:637

[FraserP117]

Thanks for the clarification @bvdmitri.

I have two things to discuss.

First Thing:

You seem to be saying that I can/should simply replace the following lines in test/visualization_tests_233_v2.jl:

push!(LOAD_PATH, joinpath(@__DIR__, "../ext"))
using GraphPPLGraphVizExt: GraphViz

with the simpler: using GraphViz . I did try this, it results in:

$ julia test/visualization_tests_233_v2.jl
ERROR: LoadError: MethodError: no method matching load(::GraphPPL.Model{MetaGraph{Int64, SimpleGraph{Int64}, GraphPPL.NodeLabel, GraphPPL.NodeData, GraphPPL.EdgeLabel, GraphPPL.Context, MetaGraphsNext.var"#4#8", Float64}, GraphPPL.PluginsCollection{Tuple{}}, RxInfer.ReactiveMPGraphPPLBackend{Static.False}}; strategy::Symbol)
The function `load` exists, but no method is defined for this combination of argument types.

Closest candidates are:
  load(::IO) got unsupported keyword argument "strategy"
   @ GraphViz ~/.julia/packages/GraphViz/IsUMl/src/GraphViz.jl:90
  load(::FileIO.File{FileIO.DataFormat{:DOT}}) got unsupported keyword argument "strategy"
   @ GraphViz ~/.julia/packages/GraphViz/IsUMl/src/GraphViz.jl:89

Stacktrace:
 [1] top-level scope
   @ ~/Desktop/GraphPPL.jl/test/visualization_tests_233_v2.jl:50
in expression starting at / ... /test/visualization_tests_233_v2.jl:50

Thus it seems that Julia is unable to find the overloaded version of GraphViz.load if I don't explicitly use

using GraphPPLGraphVizExt: GraphViz

to specify that I want my overlaoded version from GraphPPLGraphVizExt. In order to do this, I think I need to append the extension to Julia's LOAD_PATH variable. That is why I have:

push!(LOAD_PATH, joinpath(@__DIR__, "../ext"))

My understanding is that it is necessary to append the ext directory to Julia's current list of available directories in the LOAD_PATH variable. Perhaps there is a much better/cleaner way to do this.

Second Thing:

When I attempt to run my existing test script:

using GraphPPL
using Test
using RxInfer
using Distributions
using Random
using Graphs
using MetaGraphsNext
using Dictionaries
using Cairo
using Fontconfig
using Compose
using GraphPlot

push!(LOAD_PATH, joinpath(@__DIR__, "../ext"))

using GraphPPLGraphVizExt: GraphViz

## CREATE AN RXINFER.JL MODEL:
# GraphPPL.jl export `@model` macro for model specification
# It accepts a regular Julia function and builds an FFG under the hood
@model function coin_model(y, a, b)
    # We endow θ parameter of our model with some prior
    θ ~ Beta(a, b)

    # We assume that outcome of each coin flip is governed by the Bernoulli distribution
    for i in eachindex(y)
        y[i] ~ Bernoulli(θ)
    end
end

# condition the model on some observed data
conditioned = coin_model(a = 2.0, b = 7.0) | (y = [ true, false, true ], )

# `Create` the actual graph of the model conditioned on the data
rxi_model = RxInfer.create_model(conditioned)

gppl_model = RxInfer.getmodel(rxi_model)

# generate visualization:
gen_dot_result_coin_simple = GraphViz.load(
    gppl_model, 
    strategy = :simple
)

println(gen_dot_result_coin_simple)

Julia can't seem to find the prettyname function:

$ julia test/visualization_tests_233_v2.jl
ERROR: LoadError: UndefVarError: `prettyname` not defined in `GraphPPL`
Stacktrace:
 [1] get_displayed_label(properties::GraphPPL.FactorNodeProperties{GraphPPL.NodeData})
   @ GraphPPLGraphVizExt ~/Desktop/GraphPPL.jl/ext/GraphPPLGraphVizExt.jl:377
 [2] add_nodes!(io_buffer::IOBuffer, model_graph::GraphPPL.Model{MetaGraph{Int64, SimpleGraph{Int64}, GraphPPL.NodeLabel, GraphPPL.NodeData, GraphPPL.EdgeLabel, GraphPPL.Context, MetaGraphsNext.var"#4#8", Float64}, GraphPPL.PluginsCollection{Tuple{}}, RxInfer.ReactiveMPGraphPPLBackend{Static.False}}, global_namespace_dict::Dict{Int64, Dict{Symbol, Any}}, ::GraphPPLGraphVizExt.SimpleIteration)
   @ GraphPPLGraphVizExt ~/Desktop/GraphPPL.jl/ext/GraphPPLGraphVizExt.jl:423
 [3] load(model_graph::GraphPPL.Model{MetaGraph{Int64, SimpleGraph{Int64}, GraphPPL.NodeLabel, GraphPPL.NodeData, GraphPPL.EdgeLabel, GraphPPL.Context, MetaGraphsNext.var"#4#8", Float64}, GraphPPL.PluginsCollection{Tuple{}}, RxInfer.ReactiveMPGraphPPLBackend{Static.False}}; strategy::Symbol, font_size::Int64, edge_length::Float64, layout::String, overlap::Bool, width::Float64, height::Float64, show::Bool, save_to::Nothing)
   @ GraphPPLGraphVizExt ~/Desktop/GraphPPL.jl/ext/GraphPPLGraphVizExt.jl:662
 [4] top-level scope
   @ ~/ ... /test/visualization_tests_233_v2.jl:48
in expression starting at / ... /test/visualization_tests_233_v2.jl:48

I have tried all kinds of things to resolve this issue, none have worked.

Perhaps I am systematically misunderstanding how Julia loads various modules and files for actual execution and this is the reason why I'm having all this difficulty. That is my guess! I apologise for any such deficiency on my behalf.

I did actually get an overloaded version of GraphViz.load working in my own branch (minus the prettyname) so I'm not sure why there are all these difficulties here.

I hope this is enough to go on for general comments and suggestions.

[bvdmitri]

My understanding is that it is necessary to append the ext directory to Julia's current list of available directories in the LOAD_PATH variable. Perhaps there is a much better/cleaner way to do this.

No. It shouldn't be necessary and this is not how extensions in Julia work. I would think that something is wrong with your setup. You install GraphViz package and simply do using GraphViz. The extension should load automatically as soon as GraphViz is installed. That is exactly how extensions are supposed to work for a user. If this is doesn't work then something is wrong and it must be fixed, but you shouldn't workaround it by changing LOAD_PATH (and no regular user would do that). We use extensions in ReactiveMP and RxInfer and we never needed to modify the default LOAD_PATH for this feature to work properly.

ERROR: LoadError: UndefVarError: prettyname not defined in GraphPPL

I'm not sure why you get this error, because it is defined. Did you change the branch to this one when experimenting? How did you experiment? In VSCode? If not, do you have Revise.jl installed on your system that automatically reloads the code upon chaning?

[FraserP117]

Many thanks @bvdmitri

I have clearly misunderstood the nature of Julia extensions. Like you, I suspect that my setup is the issue in all this. I've been checking out your graphviz-suggestions branch.

Regarding my "setup": I don't use VSCode, I simply use the Sublime Text editor to write/edit and then use my terminal to execute the source file in question. Perhaps this smooth-brained approach is the issue after all?

I've also not been using Revise.jl, though this does sound very good - I had not heard of this until now. So for reference, what I've been doing is simply executing the following test script of mine in a terminal session.

If I remove all the include and LOAD_PATH nonsense and simply attempt using GraphViz, this is what my test file looks like:

using GraphPPL
using Test
using RxInfer
using Distributions
using Random
using Graphs
using MetaGraphsNext
using Dictionaries
using Cairo
using Fontconfig
using Compose
using GraphPlot

# using GraphPPLGraphVizExt: GraphViz
using GraphViz


## CREATE AN RXINFER.JL MODEL:
# GraphPPL.jl export `@model` macro for model specification
# It accepts a regular Julia function and builds an FFG under the hood
@model function coin_model(y, a, b)
    # We endow θ parameter of our model with some prior
    θ ~ Beta(a, b)
    # or, in this particular case, the `Uniform(0.0, 1.0)` prior also works:
    # θ ~ Uniform(0.0, 1.0)

    # We assume that outcome of each coin flip is governed by the Bernoulli distribution
    for i in eachindex(y)
        y[i] ~ Bernoulli(θ)
    end
end

# condition the model on some observed data
conditioned = coin_model(a = 2.0, b = 7.0) | (y = [ true, false, true ], )

# `Create` the actual graph of the model conditioned on the data
rxi_model = RxInfer.create_model(conditioned)

gppl_model = RxInfer.getmodel(rxi_model)


# Generate the DOT code:
gen_dot_result_coin_simple = GraphViz.load(
    model_graph = gppl_model,
    strategy = :simple
)

println(gen_dot_result_coin_simple)

And this is the result:

$ julia test/visualization_tests_233_v2.jl
ERROR: LoadError: MethodError: no method matching load(; model_graph::GraphPPL.Model{MetaGraph{Int64, SimpleGraph{Int64}, GraphPPL.NodeLabel, GraphPPL.NodeData, GraphPPL.EdgeLabel, GraphPPL.Context, MetaGraphsNext.var"#4#8", Float64}, GraphPPL.PluginsCollection{Tuple{}}, RxInfer.ReactiveMPGraphPPLBackend{Static.False}}, strategy::Symbol)
The function `load` exists, but no method is defined for this combination of argument types.

Closest candidates are:
  load(::IO) got unsupported keyword arguments "model_graph", "strategy"
   @ GraphViz ~/.julia/packages/GraphViz/IsUMl/src/GraphViz.jl:90
  load(::FileIO.File{FileIO.DataFormat{:DOT}}) got unsupported keyword arguments "model_graph", "strategy"
   @ GraphViz ~/.julia/packages/GraphViz/IsUMl/src/GraphViz.jl:89

Stacktrace:
 [1] top-level scope
   @ ~/ ... /GraphPPL.jl/test/visualization_tests_233_v2.jl:43
in expression starting at / ... /GraphPPL.jl/test/visualization_tests_233_v2.jl:43

Clearly the extension isn't being loaded correctly. This initial difficulty is what motivated my use of include and the modification of LOAD_PATH. The various packages such as GraphViz.jl are installed on my system so I can't see that as the issue.

I do apologies for my lack of understanding on this front. I'm unsure what do do from here. The last thing I want to do is cause difficulties and I fully intend to make whatever changes to my development setup that are necessary!

[bvdmitri]

@FraserP117 How did you install GraphPPL in your environment? Perhaps it simply picks-up the global version instead of your local one? In the directory where you run your script you can do the following commands:

cd directory_with_experiments
julia --project=.

and then

julia> ] # This command will start the Pkg mode, the color should change from green to blue and you should see the env name
pkg> dev path/to/local/GraphPPL.jl # dev command install the develop version of GraphPPL and accepts the path to local folder

you can also do it globally for the entire Julia by just running the same commands in julia without --project=. command.

And you should definitely install Revise.jl, it simplifies the process of development by a lot and you don't need to restart you Julia sessions all the time or running them as scripts (though its also possible, just slow). I usually use VSCode and as far as I know it has Revise.jl installed automatically.

[FraserP117]

Thanks @bvdmitri

I believe that I have resolved all my own development workflow issues and I'd like to approve this PR to resume work on the relevant branch.

You were right, I was simply using the global version of GraphPPL - silly me. I have used dev to install my local version to the "project" as specified by julia --project=.. I'm also using Revise.jl now, thanks for that tip!

I can switch over to using VSCode, if that will prove more convenient for you folks. I'll stick with Sublime Text in lieu of any recommendation to the contrary.

Having played around with this new version of the overloaded GraphViz.load function, there seems to be a small issue when specifying a location to save a rendered image with save_to. The save_to argument seems to want an absolute path, it only works for me in that case. I'll see about addressing this back on the graphviz_issue_233 branch when this PR goes through.

I need to take a look at the dependencies for using the show argument also. At present, I can't generate a plot with show = true. I hope to fix that back in the graphviz_issue_233 branch also.

Thanks for your patience, I know I've tested it!

[bvdmitri]

No problem @FraserP117 , happy to help!