Bug in MPE implementation

[yoojungchoi]

The top-down pass to get MPE states maximizes the marginals of children nodes rather than their MPE values. In fact, there is no bottom-up maximizing pass.

https://github.com/Juice-jl/ProbabilisticCircuits.jl/blob/af25169b9715c857a3cdb4d7b7345f7e27c0d425/src/Probabilistic/ProbCircuits.jl#L505

The current test cases fail to catch this bug--it turns out the branch with the largest MPE value often has the largest marginal as well.

[guyvdbroeck]

Great catch. Let's first introduce a unit test that catches this bug, before we fix it.

[khosravipasha]

Yeah the tests were not good enough to catch this. The circuit used was too simple.
We discussed with YooJung and Nikil over email, they have the correct implementations, and we have counterexamples for unit tests, for temporary fix maybe we can push those.

MPE currently is using the old flow codes and they are going away in v0.2, probably would need to reimplement MPE. This isssue is around mostly so we don't forget about the bug.

[guyvdbroeck]

I have removed the incorrect MPE implementation. I will close this bug report and make a new one asking to implement MPE from scratch.

[guyvdbroeck]

Somehow this bug was not fixed...
there is now at least a unit test to warn us about MAP being broken.

[guyvdbroeck]

The bug seems to be properly fixed now: 028d90c