Tractables / ProbabilisticCircuits.jl

Probabilistic Circuits from the Juice library
https://tractables.github.io/ProbabilisticCircuits.jl/dev
Apache License 2.0
104 stars 11 forks source link

overhaul of input-output functionality #87

Closed guyvdbroeck closed 3 years ago

guyvdbroeck commented 3 years ago

I used the occasion for some general cleanup:

codecov-commenter commented 3 years ago

Codecov Report

Merging #87 (1061b7d) into master (d456ce6) will increase coverage by 0.96%. The diff coverage is 72.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #87      +/-   ##
==========================================
+ Coverage   69.05%   70.02%   +0.96%     
==========================================
  Files          45       37       -8     
  Lines        3490     2802     -688     
==========================================
- Hits         2410     1962     -448     
+ Misses       1080      840     -240     
Impacted Files Coverage Δ
src/ProbabilisticCircuits.jl 100.00% <ø> (ø)
src/factor_graph/factor_graph.jl 100.00% <ø> (ø)
src/factor_graph/fg_compile.jl 56.41% <ø> (ø)
src/io/ensemble_io.jl 0.00% <0.00%> (ø)
src/io/plot.jl 0.00% <ø> (ø)
src/param_bit_circuit.jl 93.10% <ø> (-1.64%) :arrow_down:
src/parameters.jl 61.22% <0.00%> (ø)
src/io/clt_io.jl 73.68% <73.68%> (ø)
src/io/io.jl 88.88% <88.88%> (ø)
src/io/jpc_io.jl 93.15% <93.15%> (ø)
... and 16 more

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 d456ce6...1061b7d. Read the comment docs.

guyvdbroeck commented 3 years ago

I agree it would be useful to already move the commented-out slow tests to the slow test functionality.

For the logistic circuits, it didn't make sense to wait because the logistic circuit parser was removed from LogicCircuits.jl, and without it the expectations etc. don't work anyway. Better bite the bullet, and check out the older commit when making LogisticCircuits.jl in future. Also, who is maintaining the logistic circuit functionality? In the past we have never had that stuff functioning in Juice.

khosravipasha commented 3 years ago

Sounds good, we can merge this then. I can make the new package. I only wrote the parser to read logistic circuits. I think yitao wrote the paramter learning, we don't support anything else yet for logistic/regression circuits.