Skip to content

fix: register each anonymous variable under a unique context key - #325

Open
bvdmitri wants to merge 1 commit into
mainfrom
fix/311-anonymous-unique-keys
Open

bvdmitri wants to merge 1 commit into
mainfrom
fix/311-anonymous-unique-keys

Conversation

@bvdmitri

Copy link
Copy Markdown
Member

Closes #311.

Problem

Every anonymous variable was registered in its Context under the constant key
VariableNameAnonymous = :anonymous_var_graphppl. Context's setindex! routes that to
set!(c.individual_variables, key, val), which is insert-or-update, so when a context creates two or
more anonymous variables (e.g. the x + 1 and y + 1 sub-terms of z ~ Normal(x + 1, y + 1)) each
one silently overwrote the previous. The registry kept only the last.

The factor graph itself was always correct — each anonymous variable is a distinct vertex with the
right edges — so this was latent. But anything enumerating individual_variables(context) (notably
VarDict / getvardict) silently missed the earlier ones.

Change

Added add_anonymous_node!, mirroring the existing add_constant_node!, which registers under
to_symbol(VariableNameAnonymous, label.global_counter) and returns the label explicitly. The four
call sites in materialize_anonymous_variable! now use it.

Constants already work exactly this way (constvar_10, constvar_13, ...), which is what made the
anonymous keying look unintended in the first place.

Compatibility

The node property name is unchanged, so nothing that identifies anonymous variables breaks:

  • is_anonymous (graph_engine.jl:737) reads VariableNodeProperties.name
  • as_variable(VariableNameAnonymous) filtering compares getname(getproperties(...)), not the key

Both are property-based, so all 14 existing assertions in test/graph_construction_tests.jl pass
untouched. Nothing in src/ or ext/ looks up the literal :anonymous_var_graphppl key.

The one visible change: VarDict / getvardict and Context's show now list one entry per
anonymous variable instead of one in total, and haskey(context, :anonymous_var_graphppl) is now
false. This is the intended fix — those entries were being lost — and it matches how constvar_N
already appears there. Worth knowing for downstream consumers that iterate VarDict.

Tests

New test item in test/graph_construction_tests.jl: a submodel creating two anonymous variables in one
context, asserting the number of distinct anonymous keys in individual_variables equals the number of
anonymous vertices in the graph, and that is_anonymous still identifies both.

Verified it fails on main (2 failures) and passes here. Full suite on Julia 1.12:
259/259 test items, all passing.

🤖 Generated with Claude Code

Anonymous variables were all registered under the constant key
`VariableNameAnonymous`, and `Context`'s `setindex!` is insert-or-update, so a
context creating more than one anonymous variable kept only the last in
`individual_variables`. The graph itself was correct, but anything enumerating
the context registry (notably `VarDict`) silently missed the earlier ones.

Adds `add_anonymous_node!`, mirroring `add_constant_node!`, which registers
under `to_symbol(VariableNameAnonymous, label.global_counter)`. The node
property `name` is unchanged, so `is_anonymous` and
`as_variable(VariableNameAnonymous)` are unaffected.

Closes #311.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.73%. Comparing base (4cc7c6c) to head (b7c46d8).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #325      +/-   ##
==========================================
+ Coverage   90.72%   90.73%   +0.01%     
==========================================
  Files          16       16              
  Lines        2263     2267       +4     
==========================================
+ Hits         2053     2057       +4     
  Misses        210      210              

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multiple anonymous variables in one context collapse to a single context key

1 participant