Tractables / LogicCircuits.jl

Logic Circuits from the Juice library
https://tractables.github.io/LogicCircuits.jl/dev/
Apache License 2.0
48 stars 4 forks source link

Add BinaryDecisionDiagrams.jl to Juice #90

Closed RenatoGeh closed 3 years ago

RenatoGeh commented 3 years ago

Hi,

Sending this PR in preparation for https://github.com/Juice-jl/ProbabilisticCircuits.jl/pull/79.

Please let me know if the code's notation/naming doesn't follow Juice's.

Thanks

codecov[bot] commented 3 years ago

Codecov Report

Merging #90 (20979b5) into master (48359a7) will decrease coverage by 0.21%. The diff coverage is 92.79%.

Impacted file tree graph

@@            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     
Impacted Files Coverage Δ
src/LogicCircuits.jl 100.00% <ø> (ø)
src/Utils/misc.jl 85.96% <ø> (+0.96%) :arrow_up:
src/sdd/sdds.jl 97.87% <83.33%> (-2.13%) :arrow_down:
src/bdd/bdds.jl 92.89% <92.89%> (ø)
src/Utils/cubitvector.jl 33.33% <0.00%> (-16.67%) :arrow_down:
src/queries/satisfies.jl 62.24% <0.00%> (-14.29%) :arrow_down:
src/queries/satisfies_flow.jl 47.18% <0.00%> (-4.77%) :arrow_down:
src/Utils/data.jl 81.76% <0.00%> (-3.32%) :arrow_down:
src/bit_circuit.jl 81.30% <0.00%> (-1.87%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 48359a7...20979b5. Read the comment docs.

RenatoGeh commented 3 years ago

Overall looks comprenensive and well tested, thanks for the PR. Had a few questions inline, and also:

1. 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.

Yes, it's completely separate from LogicCircuits for now. I can write a compile function to convert it to a LogicCircuit as a placeholder until we can make BDD fully compatible with Juice.

2. 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.

There are some others: atmost, atleast, exactly, forget from the top of my head. We have some examples in the tests. But documentation is definitely lacking. I can add some proper doc on the BDD side. And then we can work together on the tutorials and docs.

khosravipasha commented 3 years ago

Cool, thanks, we don't have to do those two in this PR. I added issues to track them.

khosravipasha commented 3 years ago

@RenatoGeh Thanks for the rename, planning to merge this sometime tonight.