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

Improve conversions between Pyomo and Sympy expressions #2806

Merged
merged 12 commits into from
Apr 14, 2023

Conversation

jsiirola
Copy link
Member

@jsiirola jsiirola commented Apr 13, 2023

Fixes #2802, fixes #2803.

Summary/Motivation:

This PR improves support for converting Pyomo expressions to/from Sympy expressions. In particular:

  • Remove the redundant code from cnf_walker: the CNF walker now builds on the general walker from sympy_tools (eliminates a significant amount of repeated code)
  • Generate an exception when attempting to convert a Pyomo expression with an ExternalFunction expression node into a Sympy expression (fixes Sympy expressions leaking out of differentiation code #2803)
  • Remove use of private sympy attributes
  • Improve handling of NPV expressions

This also improves error checking in the core.logical_to_linear transformation. In particular, attempts to transform LogicalConstraints containing relational expressions will generate an exception (fixes #2802).

Changes proposed in this PR:

  • (see above)

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.

@codecov
Copy link

codecov bot commented Apr 13, 2023

Codecov Report

Patch coverage: 97.22% and project coverage change: +0.02 🎉

Comparison is base (96b408d) 87.01% compared to head (a1d2cb5) 87.04%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2806      +/-   ##
==========================================
+ Coverage   87.01%   87.04%   +0.02%     
==========================================
  Files         763      763              
  Lines       87276    87312      +36     
==========================================
+ Hits        75943    75998      +55     
+ Misses      11333    11314      -19     
Flag Coverage Δ
linux 84.03% <97.22%> (+<0.01%) ⬆️
osx 73.54% <97.22%> (+<0.01%) ⬆️
other 84.21% <86.11%> (+<0.01%) ⬆️
win 81.56% <97.22%> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
pyomo/core/plugins/transform/logical_to_linear.py 94.97% <85.71%> (+0.71%) ⬆️
pyomo/core/expr/cnf_walker.py 96.36% <100.00%> (+6.44%) ⬆️
pyomo/core/expr/sympy_tools.py 98.98% <100.00%> (+0.03%) ⬆️

... and 3 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

@jsiirola jsiirola merged commit e222276 into Pyomo:main Apr 14, 2023
@jsiirola jsiirola deleted the sympy-walker-updates branch April 14, 2023 18:44
jsiirola added a commit to jsiirola/pyomo that referenced this pull request Apr 18, 2023
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.

Sympy expressions leaking out of differentiation code DeveloperError: Internal Pyomo implementation error
2 participants