aimacode / aima-java

Java implementation of algorithms from Russell And Norvig's "Artificial Intelligence - A Modern Approach"
MIT License
1.56k stars 795 forks source link

GibbsAsk.ask() ignores evidence #245

Closed andredd closed 7 years ago

andredd commented 7 years ago

There might be an error in the GibbsAsk implementation. Evidence for child nodes is ignored. RejectionSampling and Likelyhoodweighting work as expected. Test follows:

    // create two nodes: parent and child with an arc from parent to child 
    RandomVariable rvParent = new RandVar("Parent", new BooleanDomain());
    RandomVariable rvChild = new RandVar("Child", new BooleanDomain());
    FullCPTNode nodeParent = new FullCPTNode(rvParent, new double[] { 0.7, 0.3 });
    new FullCPTNode(rvChild, new double[] { 0.8, 0.2, 0.2, 0.8 }, nodeParent);

    // create net
    BayesNet net = new BayesNet(nodeParent);

    // query parent probability
    RandomVariable[] rvX = new RandomVariable[] { rvParent };

    // ...given child evidence (true)
    AssignmentProposition[] propE = new AssignmentProposition[] { new AssignmentProposition(rvChild, Boolean.TRUE) };

    // sample with LikelihoodWeighting: result should be correct
    CategoricalDistribution samplesLW = new LikelihoodWeighting().ask(rvX, propE, net, 1000);
    assertEquals(0.9, samplesLW.getValue(Boolean.TRUE), 0.1);

    // sample with RejectionSampling: result should be correct
    CategoricalDistribution samplesRS = new RejectionSampling().ask(rvX, propE, net, 1000);
    assertEquals(0.9, samplesRS.getValue(Boolean.TRUE), 0.1);

    // sample with GibbsAsk: result is weird
    CategoricalDistribution samplesGibbs = new GibbsAsk().ask(rvX, propE, net, 1000);
    assertEquals(0.9, samplesGibbs.getValue(Boolean.TRUE), 0.1);
ngrj93 commented 7 years ago

After studying the gibbs sampling code and doing an extensive debugging run of the above submitted code I noted that the probability over markov blanket being calculated is over the same event regardless of the new values being assigned to Xi over the loop. I believe that is the cause of the problem although I may be wrong.

The normalized probability values being returned are [0.56, 0.24] in the case of {parent = true, child = true} for the above test case. I believe it should be [0.56, 0.06]. Could this be verified?

public static double[] mbDistribution(Node Xi,
            Map<RandomVariable, Object> event) {
    FiniteDomain fd = (FiniteDomain) Xi.getRandomVariable().getDomain();
    double[] X = new double[fd.size()];

    for (int i = 0; i < fd.size(); i++) {
        double cprob = 1.0;
        for (Node Yj : Xi.getChildren()) {
            cprob *= Yj.getCPD().getValue(
                getEventValuesForXiGivenParents(Yj, event));
        }
        X[i] = Xi.getCPD()
                .getValue(
                    getEventValuesForXiGivenParents(Xi,
                        fd.getValueAt(i), event))
                * cprob;
    }
    return Util.normalize(X);
}
ctjoreilly commented 7 years ago

I have this on my TODO list to investigate more closely. Thanks.

On Wed, Mar 22, 2017 at 6:49 AM, Nagaraj Poti notifications@github.com wrote:

After studying the gibbs sampling code and doing an extensive debugging run of the above submitted code I noted that the probability over markov blanket being calculated is over the same event regardless of the new values being assigned to Xi over the loop. I believe that is the cause of the problem although I may be wrong.

The normalized probability values being returned are [0.56, 0.24] in the case of {parent = true, child = true} for the above test case. I believe it should be [0.56, 0.06].

`public static double[] mbDistribution(Node Xi, Map<RandomVariable, Object> event) { FiniteDomain fd = (FiniteDomain) Xi.getRandomVariable().getDomain(); double[] X = new double[fd.size()];

for (int i = 0; i < fd.size(); i++) { // P(x'i|mb(Xi)) = // αP(x'i|parents(Xi)) // ∏Yj ∈ Children(Xi) // P(yj|parents(Yj)) double cprob = 1.0; for (Node Yj : Xi.getChildren()) { cprob = Yj.getCPD().getValue( getEventValuesForXiGivenParents(Yj, event)); } X[i] = Xi.getCPD() .getValue( getEventValuesForXiGivenParents(Xi, fd.getValueAt(i), event))

  • cprob; }

    return Util.normalize(X); }`

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/aimacode/aima-java/issues/245#issuecomment-288403502, or mute the thread https://github.com/notifications/unsubscribe-auth/AA0DuzQnEDp0z_mBSRLxsORAGrgJtWfzks5roSbjgaJpZM4Lhf1X .

ctjoreilly commented 7 years ago

Fix in #305.