Multiple dispatch for relational expression generation by jsiirola · Pull Request #3483 · Pyomo/pyomo

jsiirola · 2025-02-20T08:05:29Z

Fixes # .

Summary/Motivation:

NOTE: This PR includes #3551; review / merge that PR first.

This is a continuation of the work moving Pyomo expression generation to use multiple dispatch for more efficient / standardized argument processing. This migrates the relational expression generation. Overall, it provides more standardized and efficient argument processing. Overall performance on the performance test suite is improved (in particular, a 3.5% improvement in create_instance:

Changes proposed in this PR:

Move relational expression generation to use multiple dispatch
Move some definitions (enums and dispatcher utilities) into expr_common
Improved solution to managing the circular dependency between relations and numeric expressions (with better code documentation)

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:

I agree my contributions are submitted under the BSD license.
I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

jsiirola · 2025-02-20T08:06:03Z

(Do not attempt to squeeze this into the release)

pyomo/core/expr/relational_expr.py

mrmundt · 2025-03-24T14:27:58Z

pyomo/core/expr/relational_expr.py

+_lt_dispatcher = collections.defaultdict(lambda: _register_new_lt_handler)
+_lt_type_handler_mapping = _binary_op_dispatcher_type_mapping(
+    _lt_dispatcher,
+    {
+        (ARG_TYPE.NATIVE, ARG_TYPE.NATIVE): _lt_native,
+        (ARG_TYPE.NATIVE, ARG_TYPE.PARAM): _lt_any_param,
+        (ARG_TYPE.NATIVE, ARG_TYPE.OTHER): _lt_expr,
+        (ARG_TYPE.NATIVE, ARG_TYPE.INEQUALITY): _lt_expr_ineq,
+        (ARG_TYPE.NATIVE, ARG_TYPE.INVALID_RELATIONAL): _lt_invalid,
+        (ARG_TYPE.PARAM, ARG_TYPE.NATIVE): _lt_param_any,
+        (ARG_TYPE.PARAM, ARG_TYPE.PARAM): _lt_param,
+        (ARG_TYPE.PARAM, ARG_TYPE.OTHER): _lt_param_any,
+        (ARG_TYPE.PARAM, ARG_TYPE.INEQUALITY): _lt_param_ineq,
+        (ARG_TYPE.PARAM, ARG_TYPE.INVALID_RELATIONAL): _lt_invalid,
+        (ARG_TYPE.OTHER, ARG_TYPE.NATIVE): _lt_expr,
+        (ARG_TYPE.OTHER, ARG_TYPE.PARAM): _lt_any_param,
+        (ARG_TYPE.OTHER, ARG_TYPE.OTHER): _lt_expr,
+        (ARG_TYPE.OTHER, ARG_TYPE.INEQUALITY): _lt_expr_ineq,
+        (ARG_TYPE.OTHER, ARG_TYPE.INVALID_RELATIONAL): _lt_invalid,
+        (ARG_TYPE.INEQUALITY, ARG_TYPE.NATIVE): _lt_ineq_expr,
+        (ARG_TYPE.INEQUALITY, ARG_TYPE.PARAM): _lt_ineq_param,
+        (ARG_TYPE.INEQUALITY, ARG_TYPE.OTHER): _lt_ineq_expr,
+        (ARG_TYPE.INEQUALITY, ARG_TYPE.INEQUALITY): _lt_invalid,
+        (ARG_TYPE.INEQUALITY, ARG_TYPE.INVALID_RELATIONAL): _lt_invalid,
+        (ARG_TYPE.INVALID_RELATIONAL, ARG_TYPE.NATIVE): _lt_invalid,
+        (ARG_TYPE.INVALID_RELATIONAL, ARG_TYPE.PARAM): _lt_invalid,
+        (ARG_TYPE.INVALID_RELATIONAL, ARG_TYPE.OTHER): _lt_invalid,
+        (ARG_TYPE.INVALID_RELATIONAL, ARG_TYPE.INEQUALITY): _lt_invalid,
+        (ARG_TYPE.INVALID_RELATIONAL, ARG_TYPE.INVALID_RELATIONAL): _lt_invalid,
+    },
+    ARG_TYPE,
+)


I don't know if there is a "better" way to do this, but there are three versions of this map that are all "almost" the same, and it makes my little "minimize repeat code" heart sad.

emma58

Sorry for the long-delayed review... This is really pretty! A few questions, but otherwise it looks good.

pyomo/core/expr/numeric_expr.py

emma58 · 2025-03-11T21:47:24Z

pyomo/core/expr/expr_common.py

+class RELATIONAL_ARG_TYPE(enums.IntEnum, metaclass=enums.ExtendedEnumType):
+    __base_enum__ = NUMERIC_ARG_TYPE
+
+    INEQUALITY = 100
+    INVALID_RELATIONAL = 101


Should this have equality in it? Why doesn't it?

This is the list of argument types for relational expressions. As an Equality expression is not allowed to be the argument for another numeric / relational expression, we will never need to do special dispatch for it (it will fall back on getting mapped to INVALID).

pyomo/core/expr/expr_common.py

codecov · 2025-03-26T01:36:03Z

Codecov Report

Attention: Patch coverage is 90.86957% with 21 lines in your changes missing coverage. Please review.

Project coverage is 88.68%. Comparing base (a9d673d) to head (20d6067).

Files with missing lines	Patch %	Lines
pyomo/core/expr/relational_expr.py	86.27%	21 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3483      +/-   ##
==========================================
- Coverage   88.69%   88.68%   -0.01%     
==========================================
  Files         888      888              
  Lines      101886   101976      +90     
==========================================
+ Hits        90365    90439      +74     
- Misses      11521    11537      +16

Flag	Coverage Δ
builders	`26.55% <48.26%> (+<0.01%)`	⬆️
default	`84.80% <90.86%> (?)`
expensive	`33.95% <50.43%> (?)`
linux	`86.15% <90.86%> (-2.29%)`	⬇️
linux_other	`86.15% <90.86%> (-0.01%)`	⬇️
osx	`76.06% <90.86%> (+<0.01%)`	⬆️
win	`84.63% <90.86%> (-0.01%)`	⬇️
win_other	`84.63% <90.86%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…ional-multiple-dispatch

…handlers

jsiirola added 4 commits February 20, 2025 00:13

Move ARG_TYPE and base dispatcher utilities to expr_common

b7226e8

Use multiple dispatch for relational expression generation

8a93fbd

Update tests to reflect more consistent relational arg processing

ad5985c

Update imports to import from actual source

a76abac

jsiirola added 2 commits February 20, 2025 23:46

Merge branch 'main' into relational-multiple-dispatch

a7c0908

Merge branch 'main' into relational-multiple-dispatch

05297ed

blnicho requested review from blnicho and emma58 February 25, 2025 20:00

jsiirola and others added 4 commits March 1, 2025 21:01

Merge branch 'main' into relational-multiple-dispatch

f78bc94

Merge branch 'main' into relational-multiple-dispatch

e858b8f

Merge branch 'main' into relational-multiple-dispatch

7779930

Merge branch 'main' into relational-multiple-dispatch

8d1e7a5

mrmundt reviewed Mar 24, 2025

View reviewed changes

emma58 approved these changes Mar 24, 2025

View reviewed changes

jsiirola added 4 commits March 25, 2025 13:52

Merge branch 'main' into relational-multiple-dispatch

a5524e5

Addressing review comments

61c433f

Refactor to support reuse for testing relational expressions

9efb375

Rename to maintain consistency

20d6067

jsiirola added 10 commits March 31, 2025 22:56

Merge branch 'main' into relational-multiple-dispatch

f7fc053

Merge branch 'main' into relational-multiple-dispatch

99d1be3

NFC: address reviewer comment

2f5ac1f

Merge remote-tracking branch 'me/numeric_expr-test-driver' into relat…

c977e5b

…ional-multiple-dispatch

Reverse the priority for resolving generic asnumeric/mutable/invalid …

c2d60e2

…handlers

Update docs / fix grammar typo

fea56b1

Support testing exception message in the dispatch tester

69f5f4f

Add dispatcher tests for equality expressions

f1bc18f

Add testing for <= dispatcher

2aa4fcf

Comparing expressions should check Inequality strict-ness

353c9a9

jsiirola added 4 commits April 3, 2025 00:31

Add strict inequality dispatcher tests

7b636d1

Update tests to track change in inequality representation

7eec611

NFC: apply black

e1509c2

NFC: fix typo

ac5d88d

blnicho approved these changes Apr 3, 2025

View reviewed changes

Merge branch 'main' into relational-multiple-dispatch

1f5e1e7

mrmundt merged commit d329ee1 into Pyomo:main Apr 8, 2025
35 checks passed

jsiirola deleted the relational-multiple-dispatch branch April 8, 2025 19:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multiple dispatch for relational expression generation#3483

Multiple dispatch for relational expression generation#3483
mrmundt merged 29 commits intoPyomo:mainfrom
jsiirola:relational-multiple-dispatch

jsiirola commented Feb 20, 2025 •

edited

Loading

Uh oh!

jsiirola commented Feb 20, 2025

Uh oh!

Uh oh!

mrmundt Mar 24, 2025

Uh oh!

emma58 left a comment

Uh oh!

Uh oh!

emma58 Mar 11, 2025

Uh oh!

jsiirola Mar 25, 2025

Uh oh!

Uh oh!

codecov bot commented Mar 26, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

jsiirola commented Feb 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fixes # .

Summary/Motivation:

Changes proposed in this PR:

Legal Acknowledgement

Uh oh!

jsiirola commented Feb 20, 2025

Uh oh!

Uh oh!

mrmundt Mar 24, 2025

Choose a reason for hiding this comment

Uh oh!

emma58 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

emma58 Mar 11, 2025

Choose a reason for hiding this comment

Uh oh!

jsiirola Mar 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Mar 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jsiirola commented Feb 20, 2025 •

edited

Loading

codecov bot commented Mar 26, 2025 •

edited

Loading