Fix 1231

[cprudhom]

Branch fix_1231 brings the following improvements to the Knapsack module:

1. Bug fix (close [BUG] Knapsack removes valid solutions and can return false UNSAT or a wrong optimum #1231): Synchronize search trees before first propagation in PropKnapsackKatriel01
2. Complete documentation: Added Javadoc for:
   • Propagators (PropKnapsack, PropKnapsackKatriel01)
   • Data structures (13 files)
3. Test reinforcement: +492 lines of tests for KnapsackTest.java
4. Readability improvement: Renamed variables to match academic terminology

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request

[cprudhom]

#1237 must be amended accordingly

[jgFages]

full and not custom ?

[cprudhom]

You are right. This is legacy but not correct.

[ArthurGodet]

For me, modifications are OK, but as @jgFages said, I agree there is a forcePropagate() to change.

That said, the failing test should also be investigated on why it is failing

[ArthurGodet]

#1237 must be amended accordingly

Done 😉

[cprudhom]

I’ve just spent a bit of time looking into what the problem is with the test that’s failing.
At first glance, it seems that the patch provided here is necessary but not sufficient. I’ll try to put together a minimal working example, but it seems that despite a (necessary) update to the structures, there’s a deeper issue.

Given the complexity of the algorithm and the underlying structures, the fact that the original contributor cannot be easily contacted, and the time I have already spent on this issue, I am reluctant to look into it further.

I propose amending this pull request to add a comment explaining the use of the constraint in question and opening a separate issue to describe the bug.

[cprudhom]

I plunged back into the 2nd error and it seems that there is a bug in the neighbourhood.
I'll double-check in the next days.