Conversation
… into expr-remove-args-tuple
|
I do think there there is still a need / use for a more memory-efficient expression object for @bknueven's use case, but I think that should be done separately (probably through a re-thought MatrixConstraint expression object). |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4022 +/- ##
=======================================
Coverage 90.14% 90.14%
=======================================
Files 917 917
Lines 109261 109233 -28
=======================================
- Hits 98494 98472 -22
+ Misses 10767 10761 -6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| i: int | ||
| Index of the child argument to return | ||
|
|
||
| Returns: The i-th child node. |
There was a problem hiding this comment.
Feels a bit redundant to the main description 😛
There was a problem hiding this comment.
Yup - it is. But I was just copying the docstring from ExpressionBase. Fixed in both places.
| relocated_module_attribute( | ||
| 'UnaryBooleanExpression', | ||
| 'pyomo.core.expr.logical_expr.UnaryBooleanExpression', | ||
| version='6.10.1.dev0', |
There was a problem hiding this comment.
This branch predates the 6.10.1 release. <sigh>
mrmundt
left a comment
There was a problem hiding this comment.
I found two other typos, so sorry, nag nag nag.
mrmundt
left a comment
There was a problem hiding this comment.
I am officially good with this and will probably not nitpick anymore.
Fixes # .
Summary/Motivation:
@bknueven reached out a couple months ago pointing out that the recent work to standardize the internals of the expression system (and in particular, that
LinearExpressionshould more directly inherit fromSumExpression) had the indirect effect of tripling the memory used by aLinearExpressionobject:@bknueven's request was to revert the
LinerExpressionstorage model back to using a pair of lists. The challenge there is that the "pair of lists" model doesn't gracefully handle incrementally buildingLinearExpressionobjects through the normal operator overloading process (in large part because of challenges with handling constants that appear within the summation) -- and the fact that Pyomo's expression system now "automatically" recognizes and createsLinearExpressionobjects is a big part of the benefit from that previous rewrite.As a result, this PR does not re-implement the "pair of lists" data structure (nor does it resolve all the memory degradation). Instead, it proposes a compromise that removes the
_args_tuple fromMonomialTermExpression(and all other Expression objects). Storage now depends on the Expression object:_argattribute_l_argand_r_argattributes_argsattribute.This reduces the marginal memory overhead from 120 bytes/entry to 80 bytes per entry.
It also significantly improves model creation time (and marginally improves the lp and nl writers). In my opinion, the slight performance degradation in the GAMS writer is OK, as that writer needs to be completely redesigned (to bring it in line with the current LP/NL writers).
Changes proposed in this PR:
_args_attribute from all expression classesvalueshould be a property)AI-Use Disclosure
or
AI tools contributed to the development of this PR
Review process (select ONE):
Notes for reviewers (optional):
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: