Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for infix Boolean logical operators #2835

Merged
merged 6 commits into from
May 22, 2023

Conversation

jsiirola
Copy link
Member

@jsiirola jsiirola commented May 21, 2023

Fixes # .

Summary/Motivation:

The initial implementation of the logical expression system intentionally omitted most infix "Boolean" operators because the "attractive" operators in Python had precedence that differs from their use in logical expressions. In particular, using the shift operator (>>) for implication caused problems. However, using bitwise operators (&, |, and ^) for BooleanVar / logical expressions does not have that problem.

This PR adds infix implementations for AND, OR, and XOR operations on BooleanValue objects (and thus all logical expressions)

This PR does NOT implement __eq__, or __ne__ (although a later PR probably should ... and will be needed for #2541)

Changes proposed in this PR:

  • Add implementations for __and__, __or__, and __xor__ (along with their reversed and inplace versions) for BooleanValue objects
  • Update docs / tests

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:

  1. I agree my contributions are submitted under the BSD license.
  2. 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 jsiirola requested review from emma58 and blnicho May 21, 2023 05:34
@codecov
Copy link

codecov bot commented May 21, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.01 ⚠️

Comparison is base (dca0388) 87.19% compared to head (744dfcd) 87.19%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2835      +/-   ##
==========================================
- Coverage   87.19%   87.19%   -0.01%     
==========================================
  Files         765      765              
  Lines       88471    88504      +33     
==========================================
+ Hits        77146    77171      +25     
- Misses      11325    11333       +8     
Flag Coverage Δ
linux 84.26% <100.00%> (+<0.01%) ⬆️
osx 73.92% <100.00%> (+0.01%) ⬆️
other 84.43% <100.00%> (+<0.01%) ⬆️
win 81.80% <100.00%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
pyomo/core/expr/boolean_value.py 76.80% <100.00%> (+8.32%) ⬆️
pyomo/core/expr/logical_expr.py 90.68% <100.00%> (+0.98%) ⬆️

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

NFC: Fixing typo
Copy link
Contributor

@emma58 emma58 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheeee, thank you for this! It is indeed much prettier syntax.

We omit support for most infix operators, e.g. :code:`Y[1] >> Y[2]`, due to concerns about non-intuitive Python operator precedence.
We omit support for some infix operators, e.g. :code:`Y[1] >> Y[2]`, due to concerns about non-intuitive Python operator precedence.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually... Do we need this line of documentation at all? Isn't it just telling mice about cookies we're never planning on baking anyway?

@mrmundt mrmundt merged commit e3232d2 into Pyomo:main May 22, 2023
@jsiirola jsiirola deleted the infix-boolean-ops branch August 28, 2023 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants