Add support for templatized models, and special handling in gurobi_direct_v2#3362
Add support for templatized models, and special handling in gurobi_direct_v2#3362blnicho merged 65 commits intoPyomo:mainfrom
gurobi_direct_v2#3362Conversation
| def args(self): | ||
| # Note that it is faster to just generate the expression from | ||
| # scratch than it is to clone it and replace the IndexTemplate objects | ||
| self.set_value(self.parent_component().rule(self.parent_block(), self.index())) | ||
| return self._args_ | ||
|
|
||
| def template_expr(self): | ||
| return self._args_ | ||
|
|
||
| def set_value(self, expr): | ||
| self.__class__ = ObjectiveData | ||
| return self.set_value(expr) |
There was a problem hiding this comment.
The repeated code irks me but I don't have an immediate suggestion for how to get around it besides a MixIn class.
blnicho
left a comment
blnicho
left a comment
There was a problem hiding this comment.
Is there a reason why you didn't add template support for named Expression components as part of this PR?
| ): | ||
| super().__init__(subexpression_cache, var_map, var_order, sorter, var_recorder) | ||
| if wrt is None: | ||
| raise ValueError("ParameterizedLinearRepn: wrt not specified") |
emma58
left a comment
emma58
left a comment
There was a problem hiding this comment.
A few questions below, but mostly this looks good. Please know that my tardiness in reviewing is not correlated with how excited I am about this PR--it makes me very very happy! :)
| try: | ||
| if self._component()[self._index] is self: | ||
| return self._index | ||
| except: | ||
| pass | ||
| if self._index is NOTSET: | ||
| return self._index |
There was a problem hiding this comment.
Just out of curiosity, but why this change? Wouldn't the try-except be slower?
| # FIXME: This is a hack to get certain complex cases to print without error | ||
| _ToStringVisitor._leaf_node_types.add(TemplateSumExpression) |
There was a problem hiding this comment.
How hard is this to fix? What are the complex cases?
There was a problem hiding this comment.
I honestly don't remember (it was several months ago). I do remember fighting with it for a night and landing on this as a "reasonable" solution.
| raise NonConstantExpressionError() | ||
| raise NonConstantExpressionError( | ||
| f"{node} ({type(node).__name__}) is not fixed" | ||
| ) | ||
| if not node.is_constant(): | ||
| raise FixedExpressionError() | ||
| raise FixedExpressionError( | ||
| f"{node} ({type(node).__name__}) is not constant" | ||
| ) |
There was a problem hiding this comment.
Are these error types right? They seem flipped?
There was a problem hiding this comment.
I was confused, too: that is why I added the text description. I thought about changing it, but it has been that way since PR #467, so I was hesitant to change a "public" API (even one that is not used). I believe that the original meaning was that if you are evaluating an expression that is supposed to be constant and run across a fixed variable, then you get a FixedExpressionError. Otherwise, if you run across a free variable, then you get a generic NonConstantExpressionError.
IMHO, the long-term solution is to remove that walker.
| deprecation_warning( | ||
| "var_map, var_order, and sorter are deprecated arguments to " | ||
| "LinearRepnVisitor(). Please pass the VarRecorder object directly.", | ||
| version='6.7.4.dev0', | ||
| ) |
| ): | ||
| super().__init__(subexpression_cache, var_map, var_order, sorter, var_recorder) | ||
| if wrt is None: | ||
| raise ValueError("ParameterizedLinearRepn: wrt not specified") |
There was a problem hiding this comment.
As in: "Parameterized with respect to..." (In past Emma's defense. :))
| # Making these methods class attributes so that others can change the hooks | ||
| _get_visitor = LinearRepnVisitor | ||
| _to_vector = None | ||
| _csc_matrix = None | ||
| _csr_matrix = None |
There was a problem hiding this comment.
Wheeee, thank you! (I forgot how many amazing things there are in this PR! :))
5 participants
Fixes # .
Summary/Motivation:
This adds support for generating models with "templatized" objectives and constraints. In most cases, this does not save any net time (time saved in model construction is moved to the writer). This also introduces an extension to the
contrib.solver.gurobi_directthat recognizes and takes advantage of the expression templates. For a large model, this reduces the Pyomo overhead by roughly a third.Changes proposed in this PR:
contrib/solver/gurobi_direct.pyto work with templated modelsLegal Acknowledgement
By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution: