ReactiveMP.jl
ReactiveMP.jl copied to clipboard
Add `getnodefn` function in rules
This PR adds a new feature which allows to request a reference to a node function from the @rule
macro. In addition @call_rule
and @call_marginalrule
now attempt to create a node object when calling the specified rule.
Codecov Report
Base: 76.82% // Head: 76.75% // Decreases project coverage by -0.07%
:warning:
Coverage data is based on head (
2381b93
) compared to base (996726d
). Patch coverage: 61.36% of modified lines in pull request are covered.
Additional details and impacted files
@@ Coverage Diff @@
## master #217 +/- ##
==========================================
- Coverage 76.82% 76.75% -0.08%
==========================================
Files 203 203
Lines 6711 6738 +27
==========================================
+ Hits 5156 5172 +16
- Misses 1555 1566 +11
Impacted Files | Coverage Δ | |
---|---|---|
src/ReactiveMP.jl | 66.66% <ø> (ø) |
|
src/nodes/gamma_mixture.jl | 4.00% <7.69%> (-0.31%) |
:arrow_down: |
src/node.jl | 87.12% <62.50%> (-0.45%) |
:arrow_down: |
src/rule.jl | 55.09% <90.00%> (+0.42%) |
:arrow_up: |
src/nodes/normal_mixture.jl | 90.09% <92.30%> (+1.62%) |
:arrow_up: |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
@albertpod @nimrais @abpolym This PR is important for the upcoming delta node implementation as it changes the way we work with the node function. However, all rules would need a small update if this is merged, so I would probably prefer for all tests to be completed. But the issue is big enough and we must not release the delta branch before this one is merged.
The underlying issue is that the current approach with the DeltaFn{f}
does not work if f
depends on some global variable that are defined outside of its local scope, e.g.
y = 1.0
f = (x) -> x + y
This simply breaks Julia with undefined reference
error. The solution is this PR which forces each rule in delta node to write:
f = getnodefn(:out) # for delta forward function
f = getnodefn(:in, k) # or for backward node function
# as a feature I also added
f = getprodfn() # returns a node function defined as a pdf for stochastic nodes
This should solve the undefined reference
error, but these getnodefn
are not implemented for the delta node in this PR and must be implemented in a separate PR (or extra commit in the delta node branch).
I didn't get how you wanted to proceed with it.
Shouldn't you PR to delta
branches, after which we adopt rules and tests?
@albertpod
Yeah, I'm not entirely sure. I was thinking that it might be easier to first merge this PR to the master branch and then sync both delta branches with the master. This PR does not break anything in the master, so it can be merged. But you should be aware of the fact that it is breaking for the delta nodes and syncing with the master
branch after this PR should be performed carefully (I will help).