Refactor linear/quadratic expression compilers#3651
Conversation
… invalid argument
- fix BeforeChildDispatcher inheritance - leverage constant_flag / multiplier_flag for checking coefficients
…ize var_order dict
- changing argument ordering - more consistent handling of 0*expressions and quadratic results
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3651 +/- ##
==========================================
+ Coverage 89.08% 89.18% +0.09%
==========================================
Files 891 890 -1
Lines 102899 102774 -125
==========================================
- Hits 91671 91660 -11
+ Misses 11228 11114 -114
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:
|
| return val | ||
| return 2 # something not 0 or 1 | ||
|
|
||
| @staticmethod | ||
| def multiplier_flag(val): | ||
| if val.__class__ in native_numeric_types: | ||
| if not val: | ||
| return 2 | ||
| return val | ||
| return 2 # something not 0 or 1 |
There was a problem hiding this comment.
Should these both return 2?
There was a problem hiding this comment.
Sure. It really doesn't matter. The point (as indicated by the comment) is that it is just anything other 0 or 1. Because "2" is not unique (the val could actually be float(2)), there really isn't a driver to differentiate between 0, 2, and non-native numeric types.
| self.assertIsNone(repn.quadratic) | ||
| self.assertEqual(repn.quadratic, None) |
There was a problem hiding this comment.
Why not just use assertIsNone?
There was a problem hiding this comment.
This was part of leftover debugging. The error you get when repn.quadratic is not None is remarkable unhelpful, whereas using assertEqual gives an error message that says what repn.quadratic actually is.
github-project-automation
bot
moved this from Todo
to Reviewer Approved
in August 2025 Release
emma58
left a comment
emma58
left a comment
There was a problem hiding this comment.
I like this very much! A few questions in the comments, but I think this looks good (and I've actually been using it for awhile because I raided the cookie jar...)
github-project-automation
bot
moved this from Reviewer Approved
to Done
in August 2025 Release
5 participants
Fixes # .
Summary/Motivation:
The linear / quadratic / parameterized / templated expression walkers and compilers contain a large amount of repeated code. This PR refactors the walkers so that the bulk of repeated code can be deleted. The key changes are:
Other changes in this PR
@disable_methodsdecorator). This follows the pattern used elsewhere (e.g., in Constriant)While this PR will slow down the LP writer a little (1-2%), it resolves enough issues in the Parameterized and Templatized walkers that I think we should adopt it. There are a number of future developments in the works (that build on this PR) that should more than make up for this performance hit [moving the LP writer to leverage the standard form compiler to eliminate the need for sorting in the lp_writer and the use of dicts in the compiler for coefficient deduplication).
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: