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

Reorganize numeric expression modules #2909

Merged
merged 18 commits into from
Jul 21, 2023
Merged

Conversation

jsiirola
Copy link
Member

Fixes # .

Summary/Motivation:

This PR starts the process of reorganizing the numeric expression system to reduce the complexity induced by circular imports in the current implementation. This is the first step in a larger update of the expression systems (working toward implementing multiple dispatch for relational and logical expressions).

Changes proposed in this PR:

  • move exception definitions to pyomo.common.errors
  • move value and check_if_numeric_type to pyomo.common.numeric_types
  • move the NumericValue and NumericNDArray classes into expr.numeric_expr
  • move helper functions (trig, Expr_it) from expr.current into expr.numeric_expr
  • use attempt_import to break import loops
  • (unrelated) silence the console output for some 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 mentioned this pull request Jul 14, 2023
@codecov
Copy link

codecov bot commented Jul 14, 2023

Codecov Report

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

Comparison is base (25db781) 87.81% compared to head (5a3af38) 87.81%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2909      +/-   ##
==========================================
- Coverage   87.81%   87.81%   -0.01%     
==========================================
  Files         770      770              
  Lines       89626    89642      +16     
==========================================
+ Hits        78701    78715      +14     
- Misses      10925    10927       +2     
Flag Coverage Δ
linux 84.85% <93.35%> (+<0.01%) ⬆️
osx 74.57% <93.35%> (+<0.01%) ⬆️
other 85.02% <93.35%> (+<0.01%) ⬆️
win 82.11% <93.35%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
pyomo/core/expr/numeric_expr.py 98.60% <90.86%> (-1.02%) ⬇️
pyomo/common/numeric_types.py 92.40% <93.47%> (+1.49%) ⬆️
pyomo/common/errors.py 100.00% <100.00%> (ø)
pyomo/core/base/PyomoModel.py 84.01% <100.00%> (ø)
pyomo/core/base/connector.py 91.03% <100.00%> (+0.06%) ⬆️
pyomo/core/base/expression.py 88.93% <100.00%> (ø)
pyomo/core/base/indexed_component.py 91.49% <100.00%> (+0.01%) ⬆️
pyomo/core/base/param.py 81.74% <100.00%> (+0.04%) ⬆️
pyomo/core/base/piecewise.py 87.86% <100.00%> (ø)
pyomo/core/expr/__init__.py 100.00% <100.00%> (ø)
... and 15 more

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

Copy link
Member

@blnicho blnicho left a comment

Choose a reason for hiding this comment

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

Two minor comments but otherwise this looks fine

pyomo/common/errors.py Outdated Show resolved Hide resolved
pyomo/core/expr/__init__.py Outdated Show resolved Hide resolved
@jsiirola jsiirola merged commit ade5ac0 into Pyomo:main Jul 21, 2023
@jsiirola jsiirola deleted the reorg-expr-imports branch July 21, 2023 15:09
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.

3 participants