Skip to content
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

RuleMethodError: improve debugging information #366

Open
fonsp opened this issue Oct 3, 2024 · 3 comments · May be fixed by #370
Open

RuleMethodError: improve debugging information #366

fonsp opened this issue Oct 3, 2024 · 3 comments · May be fixed by #370
Labels
enhancement New feature or request

Comments

@fonsp
Copy link

fonsp commented Oct 3, 2024

Hello! 👋

I was experimenting with error messages in RxInfer, and talking with @Nimrais we found one example where the error message could really improve.

I followed the example from https://reactivebayes.github.io/RxInfer.jl/stable/examples/basic_examples/Bayesian%20Linear%20Regression%20Tutorial/ , but I removed the . broadcasting in the definition of y:

Before:

@model function linear_regression(x, y)
    a ~ Normal(mean = 0.0, variance = 1.0)
    b ~ Normal(mean = 0.0, variance = 100.0)    
    y .~ Normal(mean = a .* x .+ b, variance = 1.0)
end

After:

@model function linear_regression(x, y)
    a ~ Normal(mean = 0.0, variance = 1.0)
    b ~ Normal(mean = 0.0, variance = 100.0)    
    y .~ Normal(mean = a * x + b, variance = 1.0)
end

Error message:

ERROR: RuleMethodError: no method matching rule for the given arguments

Possible fix, define:

@rule typeof(*)(:A, Marginalisation) (m_out::NormalMeanVariance, m_in::PointMass, meta::MatrixCorrectionTools.ReplaceZeroDiagonalEntries{TinyHugeNumbers.TinyNumber}) = begin 
    return ...
end


Stacktrace:
  [1] (::ReactiveMP.MessageMapping{…})(messages::Tuple{…}, marginals::Nothing)
    @ ReactiveMP ~/.julia/packages/ReactiveMP/G3hZ2/src/message.jl:357
  [2] as_message(message::DeferredMessage{…}, cache::Nothing, messages::Tuple{…}, marginals::Nothing)
    @ ReactiveMP ~/.julia/packages/ReactiveMP/G3hZ2/src/message.jl:235
  [3] as_message(message::DeferredMessage{Tuple{…}, Nothing, ReactiveMP.MessageMapping{…}}, cache::Nothing)
    @ ReactiveMP ~/.julia/packages/ReactiveMP/G3hZ2/src/message.jl:231
  [4] as_message(message::DeferredMessage{Tuple{…}, Nothing, ReactiveMP.MessageMapping{…}})
    @ ReactiveMP ~/.julia/packages/ReactiveMP/G3hZ2/src/message.jl:223
  [5] MappingRF
    @ ./reduce.jl:100 [inlined]
  [6] _foldl_impl(op::Base.MappingRF{…}, init::Base._InitialValue, itr::Vector{…})
    @ Base ./reduce.jl:62
  [7] foldl_impl
    @ ./reduce.jl:48 [inlined]
  [8] mapfoldl_impl
    @ ./reduce.jl:44 [inlined]
  [9] mapfoldl
    @ ./reduce.jl:175 [inlined]
 [10] foldl
    @ ./reduce.jl:198 [inlined]
 [11] (::ReactiveMP.var"#15#17"{GenericProd, CompositeFormConstraint{Tuple{…}}})(messages::Vector{AbstractMessage})
    @ ReactiveMP ~/.julia/packages/ReactiveMP/G3hZ2/src/message.jl:149
 [12] (::ReactiveMP.var"#119#120"{ReactiveMP.var"#15#17"{…}})(messages::Vector{AbstractMessage})
    @ ReactiveMP ~/.julia/packages/ReactiveMP/G3hZ2/src/variables/variable.jl:36
 [13] next_received!(wrapper::Rocket.CollectLatestObservableWrapper{…}, data::DeferredMessage{…}, index::CartesianIndex{…})
    @ Rocket ~/.julia/packages/Rocket/LrFUI/src/observable/collected.jl:103
 [14] on_next!
    @ ~/.julia/packages/Rocket/LrFUI/src/observable/collected.jl:93 [inlined]
 [15] scheduled_next!(actor::Rocket.CollectLatestObservableInnerActor{…}, value::DeferredMessage{…}, ::AsapScheduler)
    @ Rocket ~/.julia/packages/Rocket/LrFUI/src/schedulers/asap.jl:23
 [16] on_next!(subject::Subject{…}, data::DeferredMessage{…})
    @ Rocket ~/.julia/packages/Rocket/LrFUI/src/subjects/subject.jl:62
 [17] actor_on_next!
    @ ~/.julia/packages/Rocket/LrFUI/src/actor.jl:250 [inlined]
 [18] next!
    @ ~/.julia/packages/Rocket/LrFUI/src/actor.jl:202 [inlined]
 [19] on_next!
    @ ~/.julia/packages/Rocket/LrFUI/src/subjects/recent.jl:62 [inlined]
 [20] actor_on_next!
    @ ~/.julia/packages/Rocket/LrFUI/src/actor.jl:250 [inlined]
 [21] next!
    @ ~/.julia/packages/Rocket/LrFUI/src/actor.jl:202 [inlined]
 [22] on_next!
    @ ~/.julia/packages/Rocket/LrFUI/src/operators/map.jl:62 [inlined]
 [23] next!
    @ ~/.julia/packages/Rocket/LrFUI/src/actor.jl:206 [inlined]
 [24] next_received!
    @ ~/.julia/packages/Rocket/LrFUI/src/observable/combined.jl:101 [inlined]
 [25] on_next!
    @ ~/.julia/packages/Rocket/LrFUI/src/observable/combined.jl:68 [inlined]
 [26] next!
    @ ~/.julia/packages/Rocket/LrFUI/src/actor.jl:206 [inlined]
 [27] next_received!
    @ ~/.julia/packages/Rocket/LrFUI/src/observable/combined_updates.jl:72 [inlined]
 [28] on_next!
    @ ~/.julia/packages/Rocket/LrFUI/src/observable/combined_updates.jl:34 [inlined]
 [29] scheduled_next!(actor::Rocket.CombineLatestUpdatesInnerActor{…}, value::Message{…}, ::AsapScheduler)
    @ Rocket ~/.julia/packages/Rocket/LrFUI/src/schedulers/asap.jl:23
 [30] on_next!(subject::Subject{Message, AsapScheduler, AsapScheduler}, data::Message{PointMass{Vector{Float64}}, Nothing})
    @ Rocket ~/.julia/packages/Rocket/LrFUI/src/subjects/subject.jl:62
 [31] actor_on_next!(::BaseActorTrait{…}, actor::Subject{…}, data::Message{…})
    @ Rocket ~/.julia/packages/Rocket/LrFUI/src/actor.jl:250
 [32] next!
    @ ~/.julia/packages/Rocket/LrFUI/src/actor.jl:202 [inlined]
 [33] on_next!
    @ ~/.julia/packages/Rocket/LrFUI/src/subjects/recent.jl:62 [inlined]
 [34] actor_on_next!
    @ ~/.julia/packages/Rocket/LrFUI/src/actor.jl:250 [inlined]
 [35] next!
    @ ~/.julia/packages/Rocket/LrFUI/src/actor.jl:202 [inlined]
 [36] update!
    @ ~/.julia/packages/ReactiveMP/G3hZ2/src/variables/data.jl:85 [inlined]
 [37] update!(datavar::DataVariable{Rocket.RecentSubjectInstance{…}, ReactiveMP.MarginalObservable}, data::Vector{Float64})
    @ ReactiveMP ~/.julia/packages/ReactiveMP/G3hZ2/src/variables/data.jl:84
 [38] batch_inference(; model::GraphPPL.ModelGenerator{…}, data::@NamedTuple{…}, initialization::RxInfer.InitSpecification, constraints::Nothing, meta::Nothing, options::Nothing, returnvars::Nothing, predictvars::Nothing, iterations::Int64, free_energy::Bool, free_energy_diagnostics::Tuple{…}, allow_node_contraction::Bool, showprogress::Bool, callbacks::Nothing, addons::Nothing, postprocess::DefaultPostprocess, warn::Bool, catch_exception::Bool)
    @ RxInfer ~/.julia/packages/RxInfer/vPXLG/src/inference/batch.jl:304
 [39] batch_inference
    @ ~/.julia/packages/RxInfer/vPXLG/src/inference/batch.jl:96 [inlined]
 [40] #infer#244
    @ ~/.julia/packages/RxInfer/vPXLG/src/inference/inference.jl:307 [inlined]
 [41] top-level scope
    @ ~/.julia/packages/RxInfer/vPXLG/src/model/plugins/initialization_plugin.jl:225
Some type information was truncated. Use `show(err)` to see complete types.

The information that is missing in the error message is:

  • which rule was applied? (*, currently hidden in the possible fix)
  • what types was it being applied on?
  • where does this happen in the model definition? this could be:
    • what are the variable names that it was applied on? (a and x)
    • the line where it happened (y .~ Normal(mean = a * x + b, variance = 1.0))

(Base.Experimental.register_error_hint might be useful here.)

When you do something similar in base Julia, you get a MethodError, and the error message answers these three questions:

image
@Nimrais
Copy link
Member

Nimrais commented Oct 3, 2024

Also, @fonsp proposed an interesting solution to help the user here. I am not completely sure how to implement it, but the idea is the following: convert the model into a forward model, run the forward pass, and hope that an error in the forward pass would be better. The point is the following - We have some troubles with rules at the backward pass, but we have them because the model actually does not have any physical meaning (dimension sanity check is not passing).

So for this particular example the broken forward model can be something like this

@model function linear_regression(a, b, x)
    y .~ Normal(mean = a * x .+ b, variance = 1.0)
    y .~ Uninformative()
end

if you run it

results = infer(
    model          = linear_regression(a = 10.0, b = 1.0), 
    data           = (x = [1.0, 1.0, 1, 1, 1, 1],), 
    returnvars     = (y = KeepLast()),
    iterations     = 1,
    free_energy    = true
)

you will obtain the following error

ERROR: Cannot broadcast scalar inputs over an unspecified or one-dimensional return array. Did you accidentally make a statement like this: `x ~ Bernoulli(Beta.(1, 1))` without initializing `x`?
Stacktrace:
  [1] error(s::String)
    @ Base ./error.jl:35
  [2] __check_vectorized_input(::Tuple{Nothing, GraphPPL.NodeLabel})
    @ GraphPPL ~/repos/ReactiveBayes/GraphPPL.jl/src/model_macro.jl:553
  [3] macro expansion
    @ ~/repos/ReactiveBayes/GraphPPL.jl/src/model_macro.jl:609 [inlined]
  [4] macro expansion
    @ ~/repos/ReactiveBayes/RxInfer.jl/linear_model_forward_pass.jl:28 [inlined]
  [5] add_terminated_submodel!(__model__::GraphPPL.Model{…}, __context__::GraphPPL.Context, __options__::GraphPPL.NodeCreationOptions{…}, ::typeof(linear_regression), __interfaces__::@NamedTuple{}, ::Static.StaticInt{…})
    @ Main ~/repos/ReactiveBayes/GraphPPL.jl/src/model_macro.jl:724
  [6] add_terminated_submodel!(model::GraphPPL.Model{…}, context::GraphPPL.Context, options::GraphPPL.NodeCreationOptions{…}, fform::Function, interfaces::@NamedTuple{})
    @ GraphPPL ~/repos/ReactiveBayes/GraphPPL.jl/src/graph_engine.jl:2069
  [7] add_terminated_submodel!(model::GraphPPL.Model{…}, context::GraphPPL.Context, fform::Function, interfaces::@NamedTuple{})
    @ GraphPPL ~/repos/ReactiveBayes/GraphPPL.jl/src/graph_engine.jl:2065
  [8] add_toplevel_model!(model::GraphPPL.Model{…}, context::GraphPPL.Context, fform::Function, interfaces::@NamedTuple{})
    @ GraphPPL ~/repos/ReactiveBayes/GraphPPL.jl/src/graph_engine.jl:2085
  [9] create_model(callback::RxInfer.var"#24#26"{}, generator::GraphPPL.ModelGenerator{…})
    @ GraphPPL ~/repos/ReactiveBayes/GraphPPL.jl/src/model_generator.jl:97
 [10] __infer_create_factor_graph_model(generator::GraphPPL.ModelGenerator{…}, conditioned_on::@NamedTuple{})
    @ RxInfer ~/repos/ReactiveBayes/RxInfer.jl/src/model/model.jl:122
 [11] create_model(generator::RxInfer.ConditionedModelGenerator{GraphPPL.ModelGenerator{…}, @NamedTuple{…}})
    @ RxInfer ~/repos/ReactiveBayes/RxInfer.jl/src/model/model.jl:110
 [12] batch_inference(; model::GraphPPL.ModelGenerator{…}, data::@NamedTuple{}, initialization::Nothing, constraints::Nothing, meta::Nothing, options::Nothing, returnvars::KeepLast, predictvars::Nothing, iterations::Int64, free_energy::Bool, free_energy_diagnostics::Tuple{…}, showprogress::Bool, callbacks::Nothing, addons::Nothing, postprocess::DefaultPostprocess, warn::Bool, catch_exception::Bool)
    @ RxInfer ~/repos/ReactiveBayes/RxInfer.jl/src/inference/batch.jl:199
 [13] batch_inference
    @ ~/repos/ReactiveBayes/RxInfer.jl/src/inference/batch.jl:94 [inlined]
 [14] #infer#244
    @ ~/repos/ReactiveBayes/RxInfer.jl/src/inference/inference.jl:306 [inlined]
 [15] top-level scope
    @ ~/repos/ReactiveBayes/RxInfer.jl/linear_model_forward_pass.jl:34
Some type information was truncated. Use `show(err)` to see complete types.

The key part here, as we think, is ERROR: Cannot broadcast scalar inputs over an unspecified or one-dimensional return array. If somehow it points out the variable a to the user, it will be almost the solution for them. This is because if the user tries to use broadcasting here, they will obtain the following model:

@model function linear_regression(a, b, x)
    y .~ Normal(mean = a .* x .+ b, variance = 1.0)
    y .~ Uninformative()
end

And this model runs!

results = infer(
    model          = linear_regression(a = 10.0, b = 1.0), 
    data           = (x = [1.0, 1.0, 1, 1, 1, 1],), 
    returnvars     = (y = KeepLast()),
    iterations     = 1,
    free_energy    = true
)

@show results.posteriors[:y]
# 6-element Vector{NormalMeanVariance{Float64}}:
#  NormalMeanVariance{Float64}(μ=11.0, v=1.0)
#  NormalMeanVariance{Float64}(μ=11.0, v=1.0)
#  NormalMeanVariance{Float64}(μ=11.0, v=1.0)
#  NormalMeanVariance{Float64}(μ=11.0, v=1.0)
#  NormalMeanVariance{Float64}(μ=11.0, v=1.0)
#  NormalMeanVariance{Float64}(μ=11.0, v=1.0)

@bvdmitri
Copy link
Member

bvdmitri commented Oct 4, 2024

That would be nice, but GraphPPL models do not store a direction because our models are not DAGs. You can have loops in the graph structure, making it impossible to convert a model into a forward pass model. IMO its not going to work outside of a very small class of models. There is also no notion of observations. You can have data coming both from the "top" and the "bottom" of the graph structure. E.g. in this model x comes from the "top" and y comes from the "bottom". So you do "forward" pass from x to y. There is a semantic difference of course, but from the programming perspective there is no real reason to distinguish them, so why not "forward" pass towards x given y?

The original confusion comes from the fact that m_in::PointMass hides the actual underlying type of the PointMass. I think it has been done for a reason that types might be super long (e.g. MvNormalMeanCovariance{Float64, Vector{Float64}, Matrix{Float64}} and even longer), but the suggestion is supposed to be as generic as possible so it hides the parameter types.

I think the error says that the rule is not defined for something like a PointMass{<:Matrix} or a PointMass{<:Vector}, which most probably is not compatible with NormalMeanVariance, which is a univariate distribution and expects PointMass{<:Real}. This can definitely be improved by showing a better error message indeed. GraphPPL also stores the information about the expressions where nodes have been created in the @model macro (e.g. it knows that this particular Normal node has been created with the Normal(mean = a .* x .+ b, variance = 1.0) expression etc) but we don't use this information. We should.

@bvdmitri bvdmitri added the enhancement New feature or request label Oct 4, 2024
@Nimrais
Copy link
Member

Nimrais commented Oct 4, 2024

Sure, I understand that our models are not DAGs, but when you generate a forward model, you can use additional information beyond just the graph, such as information that the user provided in the inference call. Additionally, for many nodes, we have interfaces for input and output. We can define a method for each node that would determine the generative direction.

There is a semantic difference, of course, but from a programming perspective, there's no real reason to distinguish them. So why not have a "forward" pass towards x given y?

In this model, x is attached to the interface in, and y is attached to the interface out. I think we can identify all variables that are only attached to the out interfaces and never to any others. So, we can define a method for each node called getgenerativedirection that will return the name of the interface in which you believe the natural generation occurs.

@wmkouw wmkouw linked a pull request Oct 16, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants