-
Notifications
You must be signed in to change notification settings - Fork 526
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
Update PyROS Subproblem Initialization Routines #3071
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #3071 +/- ##
==========================================
+ Coverage 88.09% 88.32% +0.22%
==========================================
Files 774 776 +2
Lines 91538 93684 +2146
==========================================
+ Hits 80644 82747 +2103
- Misses 10894 10937 +43
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Feel free to add @natalieisenberg as a reviewer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look good! I like the new system for managing decision rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same general comment as before that, if PyROS is ever promoted out of contrib
, I will request that instances of master
are changed to some other more inclusive language choice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two minor things, but nothing to prevent merging
pyomo/contrib/pyros/CHANGELOG.txt
Outdated
@@ -2,6 +2,18 @@ | |||
PyROS CHANGELOG | |||
=============== | |||
|
|||
------------------------------------------------------------------------------- | |||
PyROS 1.2.9 12 Oct 2023 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the date still 12 Oct?
targets.extend( | ||
[ | ||
con | ||
for con in blk.component_data_objects( | ||
Constraint, active=True, descend_into=True | ||
) | ||
if con not in dr_eqs | ||
if not con.equality |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that con.equality
will only be True if the constraint was declared as an equality constraint. Ranged inequalities will have con.equality == False
, even if the upper and lower bounds are the same. Is that an issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As of #2206, ranged inequality constraints with identical bounds are recast to equality constraints during the preprocessing step by util.transform_to_standard_form
.
Summary/Motivation
Implement updates to the PyROS subproblem initialization routines. These updates have been observed to improve the computational performance and reliability of PyROS for both small-scale and large-scale instances.
Changes proposed in this PR:
DiscreteScenarioSet
, the solution is feasible).1E6
.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: