Skip to content

@warn in apply_constraints! throws UndefVarError: opt when a node already has a form constraint #305

Description

@docxology

Summary

In src/plugins/variational_constraints/variational_constraints_engine.jl, two defensive
branches meant to warn "node already has a functional-form constraint, skipping" instead
raise UndefVarError: opt not defined in GraphPPL because the lazy"..." message
interpolates undefined local variables opt and constraint_data.

Locations

  • Line 954apply_constraints! for MarginalFormConstraint{…IndexedVariable…}:
    @warn lazy"Node $node already has functional form constraint $(opt[:q]) applied, therefore $constraint_data will not be applied"
  • Line 969apply_constraints! for MessageFormConstraint:
    same message.

Neither function scope binds opt or constraint_data.

Impact

The branch exists to give users a clear signal that a repeated form constraint is being
skipped. Instead of that warning, the user sees a confusing error record
("Exception while generating log record … UndefVarError: opt not defined"). The skip itself
still executes, so inference is not corrupted — this is a broken diagnostic.

Repro

using GraphPPL, Distributions
import GraphPPL: @model
@model function m(xs)
    x ~ Normal(0.0, 1.0)
    xs[1] ~ Normal(x, 1.0)
end
model = GraphPPL.create_model(m()) do m, ctx
    xs = GraphPPL.datalabel(m, ctx, GraphPPL.NodeCreationOptions(kind=:data), :xs, [1.0, 2.0])
    return (xs = xs,)
end
ctx = GraphPPL.getcontext(model)
xnode = first(collect(filter(GraphPPL.as_variable(:x), model)))
GraphPPL.setextra!(model[xnode], GraphPPL.VariationalConstraintsMarginalFormConstraintKey, :some_form)
GraphPPL.apply_constraints!(model, ctx, GraphPPL.MarginalFormConstraint(GraphPPL.IndexedVariable(:x, nothing), Normal))
# => UndefVarError: opt not defined in GraphPPL  (at variational_constraints_engine.jl:954)

Suggested fix

Use bound locals, e.g.:

@warn "Node $node already has functional form constraint $(getconstraint(marginal_constraint)) applied, therefore it will not be applied"

and the analogous MessageFormConstraint variant. Add a regression test that the warning
fires and the original constraint is preserved.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions