Add BinaryDecisionDiagrams.jl to Juice #90
Conversation
Codecov Report
@@ Coverage Diff @@
## master #90 +/- ##
==========================================
- Coverage 88.30% 88.08% -0.22%
==========================================
Files 30 31 +1
Lines 2590 3197 +607
==========================================
+ Hits 2287 2816 +529
- Misses 303 381 +78
Continue to review full report at Codecov.
|
| function Diagram(v::Bool)::Diagram | ||
| α = new() | ||
| α.index, α.value = -1, v | ||
| lock(_bdd_mutex) |
There was a problem hiding this comment.
Is locks used because its possible to build the structure in parallel?
Is it needed for other caases?
There was a problem hiding this comment.
It's only for concurrency reasons.
There was a problem hiding this comment.
Sounds good. Mostly wanted to see where the concurrency happens.
src/bdd/bdds.jl
Outdated
| - `value`: terminal boolean value | ||
| - `id`: unique identifier | ||
| """ | ||
| mutable struct Diagram |
There was a problem hiding this comment.
Why not name it BinaryDecisionDiagram? Is there collision with other things we have.
Unless Diagram can be something else too I guess, so is this limiteed to BDDs only?
There was a problem hiding this comment.
You're right. Diagram can be kind of confusing. It was originally named that in the BDD.jl package, and frankly I don't know why I chose that name. How about Bdd to parallel with Juice's Sdd?
There was a problem hiding this comment.
Sure Bdd works as well. Can also have a alias of it as BinaryDecisionDiagram so both works.
There was a problem hiding this comment.
Overall looks comprenensive and well tested, thanks for the PR. Had a few questions inline, and also:
-
If I understand correctly here BDD defines structure and everything for itself seperate than LogicCircuits. Can we complile BDDs currently to LogicCircuits? Or will need to integrate that later. Just want to make sure we will add it later on if its not possible yet.
-
At some point, I want to write some docs and tutorials for both compilation and SDD and BDDs at some point, what are the main user facing function for BDDs supported with this PR, want to keep track of them (other than the obvious load an save).
reduce!,restrict,shannon,eliminate,marginalize,mentions,print_nf,normal_form. Did I miss any important one? I guess they all have examples on how to use in the tests? If yes that's enough for me later on.
Yes, it's completely separate from LogicCircuits for now. I can write a
There are some others: |
|
Cool, thanks, we don't have to do those two in this PR. I added issues to track them. |
|
@RenatoGeh Thanks for the rename, planning to merge this sometime tonight. |
Hi,
Sending this PR in preparation for Tractables/ProbabilisticCircuits.jl#79.
Please let me know if the code's notation/naming doesn't follow Juice's.
Thanks