Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to disable dual reductions for Clp #630

Merged
merged 5 commits into from
Mar 17, 2024

Conversation

patrickvossler18
Copy link
Contributor

This pull request allows users of the C interface to prevent Clp from using dual reductions during the presolving stage by specifying a reduction type when calling Cbc_solve. While it should be possible for users to disable presolving completely using Cbc_setParameter, it appears there is an issue with such settings being passed to Clp from Cbc.

Furthermore, this option is critical for users solving MIPs using lazy constraints. If one cannot disable dual reductions or disable presolving altogether then the presolving stage will delete variables such that the constraints that are inserted by the constraint handler lead to infeasibility.

Finally, to preserve backward compatibility, the reduction type is set to a default value that does not change the Clp options by default. If the maintainers have a preferred way for ensuring that the default behavior is the same, please let me know and I can modify my PR.

@CLAassistant
Copy link

CLAassistant commented Jan 3, 2024

CLA assistant check
All committers have signed the CLA.

- add a method for disabling dual reductions to Cbc_Model
- Remove reduction type argument from Cbc_solve and Cbc_solveLinearProgram
- Change logic in Cbc_solveLinearProgram to depend on the value of the red_type parameter in Cbc_Model
@patrickvossler18
Copy link
Contributor Author

Looks like the checks are failing for Windows i686 because of an issue in the setup of the github action. This seems to be happening in other COIN-OR repos too coin-or/Clp#284

@jhmgoossens
Copy link
Contributor

jhmgoossens commented Jan 13, 2024

Thanks for this PR!

Yes, the failed Windows i686 check has nothing to do with your PR (See Issue 25 on OptimizationSuite).

Looking at name of the added method "Cbc_disableDualReds" and the default option to "LPR_Default = 0, /*! Solver will use default presolve transformations */", would a more descriptive name for the method not be "Cbc_setDualReductionsType", or similar?
Also, an added test in test/CInterfaceTest.c would be nice.

I don't use the C Interface myself. Other than the name of the added method and a test, I don't have comments.
This PR offers a workaround to an existing issue. Looking at recent commits, could maybe @jjhforrest, @samuelsbrito or @tkralphs look at this PR?

@patrickvossler18
Copy link
Contributor Author

Thanks for looking through my PR. I like your suggestion for changing the function name so I've made that change, added a function to get the dual reduction value, and added a simple test to confirm that we are successfully setting reduction value.

@jhmgoossens jhmgoossens merged commit 52a59a7 into coin-or:master Mar 17, 2024
12 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants