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

Prox approx part 3 #464

Merged
merged 11 commits into from
Dec 3, 2024
Merged

Prox approx part 3 #464

merged 11 commits into from
Dec 3, 2024

Conversation

bknueven
Copy link
Collaborator

@bknueven bknueven commented Dec 2, 2024

  • Re-implement cut manager based on tolerance
  • Add cuts based on W
  • Add safety cuts if W == 0 and xbar == xhat
  • Don't bother if the variable is fixed
  • Skip cut if it is outside the variable bounds

@@ -984,7 +984,8 @@ def nonant_cost_coeffs(s):
if id(var) in s._mpisppy_data.varid_to_nonant_index:
raise RuntimeError(
"Found nonlinear variables in the objective function. "
Copy link
Collaborator

Choose a reason for hiding this comment

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

Change "Found nonlinear..." to "A call to nonant_cost_coefficient found nonlinear" (even though the user could figure that out from the stack trace, I don't want to give the impression that nonlinear is not OK in general")

@bknueven
Copy link
Collaborator Author

bknueven commented Dec 2, 2024

Not really sure if this PR or #467 should be merged instead.

PR #467 takes a more systematic approach, and hence adds more cuts. This can slow subproblem solves, but the iterations seem to be of higher quality due to the extra cuts.

@DLWoodruff
Copy link
Collaborator

Not really sure if this PR or #467 should be merged instead.

PR #467 takes a more systematic approach, and hence adds more cuts. This can slow subproblem solves, but the iterations seem to be of higher quality due to the extra cuts.

To state the obvious: we could have both and that might even be a good idea (but does require even more work)

mpisppy/utils/sputils.py Outdated Show resolved Hide resolved
@bknueven
Copy link
Collaborator Author

bknueven commented Dec 3, 2024

Not really sure if this PR or #467 should be merged instead.
PR #467 takes a more systematic approach, and hence adds more cuts. This can slow subproblem solves, but the iterations seem to be of higher quality due to the extra cuts.

To state the obvious: we could have both and that might even be a good idea (but does require even more work)

Yes. I am confident this is an improvement over main, so let's merge this. It is simpler, and then we can consider the additional changes in #467 separately.

@bknueven bknueven enabled auto-merge December 3, 2024 16:23
@bknueven bknueven merged commit 1a04420 into Pyomo:main Dec 3, 2024
17 checks passed
@bknueven bknueven deleted the prox_approx_4 branch December 3, 2024 16:26
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.

2 participants