Skip to content

PR draft — Validate captured value in ClosedForm closure extraction (fixes issue-06) - #109

Open
docxology wants to merge 1 commit into
ReactiveBayes:mainfrom
docxology:fix/closure-validation
Open

PR draft — Validate captured value in ClosedForm closure extraction (fixes issue-06)#109
docxology wants to merge 1 commit into
ReactiveBayes:mainfrom
docxology:fix/closure-validation

Conversation

@docxology

Copy link
Copy Markdown

Problem

exmplate preprocess_strategy_argument(::ClosedFormStrategy, argument::F) takes getfield(argument, first(fieldnames(F))) with no type check; multi-variable closures silently retrieve the wrong (first) captured value.

Change

ext/ClosedFormExpectationsExt/ClosedFormExpectationsExt.jl:

-    captured = getfield(argument, first(field_names))
-    return (strategy, Logpdf(captured))
+    captured = getfield(argument, first(field_names))
+    if !(captured isa Union{Distribution,ProductOf})
+        error(
+            "`ClosedFormStrategy` expected the first captured variable of the closure to be a " *
+            "`Distribution` or `ProductOf`, but got `$(typeof(captured))`. `ClosedFormStrategy` " *
+            "supports closures that capture exactly one distribution. " *
+            "Pass the `Distribution`/`ProductOf` directly to avoid ambiguity.",
+        )
+    end
+    return (strategy, Logpdf(captured))

Optionally, when more than one field is captured, prefer the field whose value is a Distribution/ProductOf and error only if none is found.

Verification

  • let a = 5, d = Normal(0,1); fn=(x)->logpdf(d,x)+a; preprocess_strategy_argument(...) now errors with a clear message instead of silently projecting the scalar 5.
  • Single-distribution closures still work.
  • Full suite green.

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.

1 participant