search

thomas-maeder / popeye

Popeye is a chess problem solving and testing software with strong support for fairy chess and heterodox genres. For more information cf. topic "Popeye (chess)" on http://en.wikipedia.org/
31 stars 14 forks source link

Impossible return value? #289

Open JoshuaGreen opened 4 years ago

JoshuaGreen commented 4 years ago

At some point when solving EXAMPLES/maff.inp, the function conditional_pipe_solve_delegate in solving/conditional_pipe.c returns 6 == next_move_has_no_solution. The comments suggest that this should be impossible.

JoshuaGreen commented 4 years ago

This might be related to void maff_immobility_tester_king_solve(slice_index) in conditions/maff/immobility_tester.c potentially setting solve_result to next_move_has_no_solution. (solve_result isn't set to next_move_has_no_solution too many places in the codebase.)

JoshuaGreen commented 4 years ago

@thomas-maeder, you certainly have a better understanding of the MAFF rule and how it's implemented via, e.g., void maff_immobility_tester_king_solve(slice_index). Nonetheless, I offer feature/fixingIssue289 as a possible fix for this issue. Feel free to merge/correct/ignore.

JoshuaGreen commented 4 years ago

(Also, if the comments regarding conditional_pipe_solve_delegate in solving/conditional_pipe.c are accurate then it might be worth adding an assert to catch any violations.)

JoshuaGreen commented 4 years ago

I'm closing this issue as I think I've fixed it in 100a3fd.

thomas-maeder commented 4 years ago

@JoshuaGreen sorry for not responding earlier. I have been quite busy since 4.85.

Please see my comment to commit 100a3fd - it seems to break Maff handling.

JoshuaGreen commented 4 years ago

@thomas-maeder, thanks for catching this. I do see that the output is different, though it will take me some time to figure out exactly what's going on and convince myself that the new output is wrong. At any rate, it seems like we need to do something here, or perhaps restore the original code and update the comment to match the actual behavior.

Sorry for trying to take so much power into my own hands. At least my fix for Issue #298 should be correct!

JoshuaGreen commented 4 years ago

Some quick testing reveals that restoring the original code but replacing next_move_has_no_solution with previous_move_has_not_solved restores the original output (which looks correct to me). @thomas-maeder, do you have any thoughts on that precise change? (It's hard for me to understand what next_move_has_no_solution is supposed to communicate.)