Closed RenatoGeh closed 4 years ago
Hi Renato; I plan to refactor things starting next week and will keep these questions in mind. Perhaps it would indeed be good to get rid of all decorator patterns and just copy structures as needed. We started moving in this direction last winter when we added data fields to all node structs to avoid flow circuit decorators.
Thanks for the reply! Good to hear the code is getting simpler.
I have a couple of additional questions if you don't mind me asking. I constructed a PSDD and now I'm trying to extract the log-likelihood of some instantiation. When I compute values by my own through a standard bottom-up evaluation, it computes fine and returns the expected probabilities. But when I try to use ProbabilisticCircuits
's log_likelihood_per_instance
, I get the following error:
ERROR: LoadError: AssertionError: Root probabilities are expected to be 1: Bool[0] ≆ true
Stacktrace:
[1] pass_up(::Array{UpFlowΔNode{ProbΔNode{UnstLogicalΔNode},BitArray{1}},1}, ::PlainXData{Bool,BitArray{2}}) at /home/renatogeh/.julia/packages/LogicCircuits/2pF03/src/Logical/UpFlowCircuits.jl:155
[2] pass_up_down(::Array{DownFlowΔNode{ProbΔNode{UnstLogicalΔNode},BitArray{1}},1}, ::PlainXData{Bool,BitArray{2}}) at /home/renatogeh/.julia/packages/LogicCircuits/2pF03/src/Logical/DownFlowCircuits.jl:219
[3] log_likelihood_per_instance(::Array{DownFlowΔNode{ProbΔNode{UnstLogicalΔNode},BitArray{1}},1}, ::PlainXData{Bool,BitArray{2}}) at /home/renatogeh/.julia/packages/ProbabilisticCircuits/c2NU1/src/Probabilistic/ProbCircuits.jl:319
[4] log_likelihood_per_instance(::Array{ProbΔNode{UnstLogicalΔNode},1}, ::PlainXData{Bool,BitArray{2}}) at /home/renatogeh/.julia/packages/ProbabilisticCircuits/c2NU1/src/Probabilistic/ProbCircuits.jl:295
...
What exactly does "root probabilities expected to be 1" mean?
I'm also trying to estimate the parameters of a prob circuit. I tried using estimate_parameters
with a Prob\Delta
and an XData
as arguments, but I ran into several errors stating there were no methods matching for component_weights
with Prob\Delta
as an argument. So I looked into the code and found estimate_parameters2
. It ran fine and seemingly increased LL as expected, but estimate_parameters2
is not exported nor used anywhere in ProbabilisticCircuits
. Should I not be using that? If so, how can I learn weights given only a prob circuit and data?
Many thanks, Renato
That error means that you used an input example for which the output of the (logical) circuit is 0/false, which is disallowed for parameter learning. In other words, the example is outside of the support of the circuit's possible distributions.
Hi Guy. Thanks for the quick answer.
Sorry, my questions were worded in a confusing way. What I mean is that the root probabilities are expected to be 1
error happens when I try to simply evaluate the network, and not during learning. Or is log_likelihood_per_instance
not simply an evaluation function? If that is the case, how does one do a log-likelihood evaluation of the network?
During learning I get a completely different error, saying that there are no methods matching component_weights
with Prob\Delta
.
Thanks again, Renato
What would be the desired output for a log_likelihood outside of the support? Perhaps negative infinity as a float?
Oh ok I see your point now. I would say that negative infinity is reasonable for an input outside the support.
I fixed the likelihood code to properly give -Inf loglikelihood when the example does not satisfy the logical circuit / is not in the support of the distribution: https://github.com/Juice-jl/ProbabilisticCircuits.jl/commit/0930ca0c6b9588908793f7dc942e5aa63e0d5739
The API for constructing circuits has also significantly improved in recent weeks ahead of the 0.2 release, so I'm closing this issue.
Hi all,
I'm a bit confused on how exactly to create
Prob\DeltaNode
s. There are no tests (or documentation) showing how to manually create a Probabilistic Circuit, and I'm basing solely on the code itself (more specificallysrc/Probabilistic/ProbCircuits.jl:Prob\Delta
and constructors). From what I understand, every time I create a Prob node, I have to add its Logic counterpart as an "origin". Is thisorigin
required to be the actual underlying Logic node (with corresponding logic children, etc.) or may it be an "empty shell"? I see thisorigin
is used on structured circuits for checking vtrees. But if I were to use only standard probabilistic circuits, would this be really necessary? If not, is there a more lightweight circuit variant where I won't have to allocate double the memory for each circuit?Many thanks, Renato