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

Feasibility pump and updates in GOA #12

Open
wants to merge 2,833 commits into
base: master
Choose a base branch
from
Open

Feasibility pump and updates in GOA #12

wants to merge 2,833 commits into from

Conversation

ZedongPeng
Copy link
Owner

Fixes # .

Summary/Motivation:

Changes proposed in this PR:

Legal Acknowledgement

By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the BSD license.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

@ZedongPeng ZedongPeng requested a review from bernalde October 28, 2020 12:45
Copy link
Collaborator

@bernalde bernalde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great PR. I just left some minor comments and a question open on how to implement the tabu list from the storms of feasibility pumps (according to the paper it is WAY better than no good cuts, so we might give it a look).
image

solve_data.LB = results['Problem'][0]['Lower bound']
elif not math.isnan(results['Problem'][0]['Upper bound']):
solve_data.UB = results['Problem'][0]['Upper bound']
# TODO: should we use the bound of the rNLP here?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the solver provides a lower bound, we could use it (in the case for example we do not trust the optimal solution of the rNLP). I think that in general, we decide to trust the bound though.

@@ -48,7 +48,10 @@ def solve_subproblem(solve_data, config):
TransformationFactory('core.fix_integer_vars').apply_to(fixed_nlp)

MindtPy.MindtPy_linear_cuts.deactivate()
# TODO: Do we really need to calculate tmp_duals by default?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good point. If we do not add the equality relaxation cuts this is time-consuming and dumb. Consider removing it.

@@ -71,7 +74,8 @@ def solve_subproblem(solve_data, config):
try:
fixed_nlp.tmp_duals[c] = c_geq * max(
0, c_geq*(rhs - value(c.body)))
except (ValueError, OverflowError) as error:
except (ValueError, OverflowError, AttributeError) as error:
# AttributeError is included to condition when value(c.body) belongs to complex type
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When does this happen!?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This happens when value(c.body) is a complex number. In some examples, due to the numerical computation, 1+0.0001j may show up.

counter += 1

config.logger.info("Added %s affine cuts" % counter)

def add_lazy_nogood_cuts(self, var_values, solve_data, config, opt, feasible=False):
def add_lazy_no_good_cuts(self, var_values, solve_data, config, opt, feasible=False):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought there was a TODO above mentioning this. Are we using no-good in single tree? Did you try the taboo list mentioned in the storm of feasibility pump paper?
A way of implementing this is their section 5.2 using https://www.ibm.com/support/knowledgecenter/SSSA5P_12.10.0/ilog.odms.cplex.help/refpythoncplex/html/cplex.callbacks.Context-class.html#reject_candidate

@@ -23,7 +23,7 @@
from pyomo.contrib.mindtpy.tests.feasibility_pump1 import Feasibility_Pump1
from pyomo.contrib.mindtpy.tests.feasibility_pump2 import Feasibility_Pump2

required_solvers = ('ipopt', 'cplex')
required_solvers = ('ipopt', 'glpk', 'cplex')
if all(SolverFactory(s).available() for s in required_solvers):
subsolvers_available = True
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will skip some of the tests of CPLEX is not available although mots of them run with GLPK.

@ZedongPeng
Copy link
Owner Author

Great PR. I just left some minor comments and a question open on how to implement the tabu list from the storms of feasibility pumps (according to the paper it is WAY better than no good cuts, so we might give it a look).
image

I am a little confused about how they handle an integer solution found by CPLEX at the root node appears in the tabu list. Will the root node make a difference?

blnicho and others added 27 commits May 17, 2021 12:36
I am pretty sure this had a syntax error (undefined rank variable) and
was never used (certainly not covered by any tests).
Convert `Disjunct.indicator_var` from binary to Boolean variable
Allow assigning None to a unitted variable
Update Pyomo book examples & tests for the 3rd edition
pyomo.network / pyomo.gdp interoperability
jsiirola and others added 29 commits June 17, 2021 12:32
Move constrained_layout tests to 'expensive' suite
Implementation of LOA single-tree
ZedongPeng pushed a commit that referenced this pull request Oct 24, 2021
Update IDAES branch to latest Pyomo master
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.

9 participants