-
Notifications
You must be signed in to change notification settings - Fork 4
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
Structured smoothing #69
Conversation
Codecov Report
@@ Coverage Diff @@
## master #69 +/- ##
==========================================
+ Coverage 82.04% 84.06% +2.02%
==========================================
Files 29 29
Lines 2389 2454 +65
==========================================
+ Hits 1960 2063 +103
+ Misses 429 391 -38
Continue to review full report at Codecov.
|
slc = smooth(sdd) | ||
plc = propagate_constants(sdd, remove_unary=true) | ||
structplc = compile(StructLogicCircuit, vtr, plc) | ||
sstructplc = smooth(structplc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to smooth it twice? Is it because propagage constants need smooth circuit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The smoothed logic circuit is totally orthogonal, it's just for testing. I'm using probabilistic equivalence checking to make sure the smoothing didn't somehow change the meaning of the circuit (that you get the same thing regardless of how you smooth).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, makes sense, I see now, initially read line 22 wrong.
This adds smoothing for structured logic circuits, respecting the current vtree. Appears to be working - looking for more example tests that don't have dependencies in Probabilistic circuits.
I couldn't find a way around doing work at both AND and OR nodes, it's possible there is some unnecessary work going on.
As of right now this will only work on logic circuits due to typing issues. A future pr should add a "structured decomposable" trait which would allow this to work with structured prob nodes as well.