New options and mappings in the core.add_slack_variables Transformation#3869
New options and mappings in the core.add_slack_variables Transformation#3869emma58 wants to merge 7 commits intoPyomo:mainfrom
Conversation
…rmation, as well as an API to retrieve slack variables and relaxed constraints from each other
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3869 +/- ##
=======================================
Coverage 89.92% 89.93%
=======================================
Files 902 902
Lines 106393 106438 +45
=======================================
+ Hits 95679 95726 +47
+ Misses 10714 10712 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jsiirola
left a comment
jsiirola
left a comment
There was a problem hiding this comment.
I don't see any problems with this PR, but I do have a couple philosophical questions...
| A constraint that was relaxed by the transformation (either | ||
| because no targets were specified or because it was a target) | ||
| """ | ||
| slack_variables = model.private_data().slack_variables |
There was a problem hiding this comment.
Do you get a meaningful error if get_slack_variables is called on a model that was not transformed by this Transformation?
There was a problem hiding this comment.
Wow, significantly more meaningful than I was expecting, actually:
>>> from pyomo.environ import *
>>> m = ConcreteModel()
>>> m.x = Var()
>>> m.y = Var()
>>> m.c = Constraint(expr=(3, m.x + m.y, 4))
>>> m.obj = Objective(expr= m.x - m.y)
>>> TransformationFactory('core.add_slack_variables').get_slack_variables(m, m.c)
Traceback (most recent call last):
File "<python-input-6>", line 1, in <module>
TransformationFactory('core.add_slack_variables').get_slack_variables(m, m.c)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^
File "/Users/esjohn/src/pyomo/pyomo/core/plugins/transform/add_slack_vars.py", line 240, in get_slack_variables
raise ValueError(
...<3 lines>...
)
ValueError: It does not appear that c is a constraint on model unknown that was relaxed by the 'core.add_slack_variables' transformation.
We were having a very smart day when we implemented the private data Blocks... This is working because, in the scope of that file, slack_variables gets created when we get the private_data, so then it's just empty and we get a good error.
| o.deactivate() | ||
|
|
||
| # make a new objective that minimizes sum of slack variables | ||
| xblock._slack_objective = Objective(expr=obj_expr) |
There was a problem hiding this comment.
Is there a use-case for saving the obj_expr in the privae_data() and adding an API for users to get it? For example, that would make it easier to add it as a penalty to the existing Objective... Without it, a user has to search the new block for a bunch of scalar variables...
There was a problem hiding this comment.
I'm open to this, but a user wouldn't have to go searching for the scalars: they can get them via the Constraints. But maybe since this is probably a relatively common use case, we should make it easier.
| trans_info.slack_variables[cons].append(posSlack) | ||
| trans_info.relaxed_constraint[posSlack] = cons |
There was a problem hiding this comment.
Philosophical question: now that we are providing a useful API for mapping slacks to constraints and back, does it make sense to avoid all the name munging and just make a single indexed xblock.slacks = Var(NonNegativeIntegers, bounds=(0, None))?
There was a problem hiding this comment.
Maaaaybe. Though the way it is now, pprint is enough for debugging... I am certainly guilty of this method.
…t will now error for an untransformed GDP
|
I decided to make this error for unknown active components. This means it will now die on an untransformed GDP, but I think that's okay because if someone wants to add slacks on a Disjunct, they can just call it on the Disjunct directly. (It just seems like that would be a rare enough use case compared to the more likely forgot-to-transform-first situation that it's okay to make it harder to accomplish.) |
4 participants
Fixes # .
Summary/Motivation:
We've always used the
core.add_slack_variablestransformation as purely a debugging tool for infeasible models. However, there are other reasons one could want to add slack variables, such as relaxing certain constraints of a model by bounded amounts. This PR adds a couple features to the add slack variables transformation to make that easier, specifically an API so that a user can get the slack variables from the constraints and adding an option to not change the objective.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: