kiselev / aic-praise

Automatically exported from code.google.com/p/aic-praise
0 stars 0 forks source link

Investigate if R_complete_simplify really needed in R_belief's partition on intersenction logic. #14

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
The return value from the function:

find_msg_value_matching_previous_message

used by R_belief, was:

R_simplify(if D then v else prev_message)

but was changed to:

R_complete_simplify(if D then v else prev_message)

for the following reasons:

1. v may be conditional and under D some of the branches may be
unreachable (due to the fact this is an intersection not a subset
relationship). I don't believe the logic in pick_single_element etc...
ensure against unreachable branches and therefore doing here seems to
make sense. 

2. If unreachable branches are not removed, in the call to
R_complete_simplify at the end of use_values_for_previous_msgs the
assertion check inside of R_lift_product_of_factor_to_variable will
throw an assertion in an unreachable branch because it won't be able
to pick_single_element in this case. 

However, the following concern was raised:

Perhaps you are right. Unreachable branches may not satisfy the
invariant of being a singleton (which would still be ok since they don't
matter), but then they get externalized and pick_single_value would try
to find a single element in them and break. 

My concern is that R_simplify should work and that by using
R_complete_simplify we are simply masking some bug. But I suppose that
if we find this reasonable, we might run the risk of masking it and
finding it later (it will crop up again if it matters).

In any case, it seems that even so, we still get trouble with
pick_single_element in LBP Stress test #4, right? So maybe it still has 
problems. If we find a bug there and fix it, we might want to try reverting to 
R_simplify to see if it works. 

Original issue reported on code.google.com by ctjoreilly@gmail.com on 4 Jun 2013 at 6:36

GoogleCodeExporter commented 8 years ago
Reverting back to using R_simplify in SVN rev=774, as other fixes applied to 
IfThenElseConditionIsTrueInThenBranchAndFalseInElseBranch and  
LiftProductOfFactorToVariable resolved this issu.

Original comment by ctjoreilly@gmail.com on 7 Jun 2013 at 9:41