-
Notifications
You must be signed in to change notification settings - Fork 4
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
Mindtpy rewrite #26
base: main_copy
Are you sure you want to change the base?
Mindtpy rewrite #26
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good first pass @ZedongPeng . We need the code to pass all tests and address all the TODOs that you left behind, but it is looking way better. I like that we are doing this step by step.
I would like to have Can and Kaiyu look into this as soon as they can such that we can have several rounds of reviews here.
pyomo/contrib/mindtpy/nlp_solve.py
Outdated
@@ -146,6 +151,7 @@ def handle_nlp_subproblem_tc(fixed_nlp, result, solve_data, config, cb_opt=None) | |||
|
|||
|
|||
def handle_subproblem_optimal(fixed_nlp, solve_data, config, cb_opt=None, fp=False): | |||
# TODO: move this to algorithm base class. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have a handle_subproblem_optimal
method in the algorithm base class. However, we still need this in the single tree implementation.
solve_data.dual_bound_progress_time = [1, 2, 3, 4, 5, 6] | ||
solve_data.dual_bound = 5 | ||
self.assertEqual(get_dual_integral(solve_data, config), 14.1) | ||
# MindtPy = solve_data.working_model.MindtPy_utils |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these commented out because they fail or do they need to be adapted to the new code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The unit test are used for functions. Now the functions becomes methods in the algorithm base class. So it doesn't work now.
pyomo/contrib/mindtpy/util.py
Outdated
@@ -42,6 +42,7 @@ class MindtPySolveData(object): | |||
""" | |||
pass | |||
|
|||
# TODO: move all functions in this file to algorithm base class and solve_data data can be removed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe not all functions. The functions that applies to all strategys and do not depend on solve_data can be kept here.
ddae27d
to
596ddb7
Compare
cecc51b
to
7ea1ea3
Compare
Clean up implicit function imports
…ation This is a stop-gap to resolve import test failures until PyNumero can be properly updated (see Pyomo#2894)
Make min and max work with MPIBlockVector when some blocks have size 0
Fixing a 'that' vs. 'which' mistake in error about bounds crossing
Remove HTML4 Forcing in Online Docs
Resolve error categorizing some Var discrete domains as "integer"
Named expressions: `expr` should always return `NumericValue`
Support kwargs in partial objects passed to Initializer()
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: