Merge Expression Branch #272
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Dec 5, 2017
1. Created a RangedExpression, and simplified the logic in InequalityExpression. Now, inequality expressions always have two arguments, and ranged expressions always have 3. This cleans up a lot of code, and it simplifies the semantics of constraint management with expressions. 2. Migrated logic for detecting chained expressions into a separate class (_chainedInequalities) rather than annotate the InequalityExpression class.
Following the update to expressions to use RangedExpression, this commit uses the _chainedInequality global data and explicitly recognizes RangedExpression objects.
Removing expressions of the form: lower <= body <= upper Using the tuple notation, which is more robust.
In the kernel, we explicitly construct a ranged expression rather than use chained inequalities.
Added tests that this generates ranged expressions.
Using the inequality() function, and updating baselines.
The bounds_to_vars transformation was modified slighty, since the standard repn doesn't include coefficients that are zero. Hence, such constraints are ignored.
If a nonmutable param is uninitialized, accessing its value returns an exception. Hence, the logic for parameter access was modified in several places.
Resolving changes due to the merge: 1. The param semantics changed with uninitialized parameters 2. A change in semantics for a transformation, based on the new representation logic.
Codecov Report
@@ Coverage Diff @@
## master #272 +/- ##
==========================================
- Coverage 65.97% 63.94% -2.04%
==========================================
Files 488 354 -134
Lines 63228 59572 -3656
==========================================
- Hits 41715 38093 -3622
+ Misses 21513 21479 -34
Continue to review full report at Codecov.
|
blnicho
previously approved these changes
Apr 26, 2018
Reordering a multiplication in the standard repn
Added another check for a simple product with a constant. This was needed after removing the tacit assumption of ordering within product expressions. This "bug" led to a significant degredation in runtime in the uc1 test problem.
1. Clarified the semantics of is_potentially_variable()
If this method returns False, then there are no variables
in the expression. But if it returns True, the expression
may still not have any variables. This allows for performance
optimizations in cases where expressions are likely to be
variable.
2. SumExpression.is_potentially_variable() returns True
This is the typical case, so this provides a modest
improvement in runtime (e.g. 3% improvement writing the
uc1 nl file).
Pyomo5 expressions print integer values when possible
Gabe has a nice example illustrating why the new semantics didn't make sense. I updated the documentation to account for John's description of this method.
|
Can someone re-review this PR so I can merge? I'm awaiting test results, but they're looking good so far. |
ghackebeil
approved these changes
Apr 26, 2018
|
@whart222: Either before or after you merge, can you remove the |
|
Thanks. I'm going to leave these in. I'm about at the point where I have clean tests, and I want to be able to merge soon. |
|
I created the branch master_at_exprdev_merge immediately before merging this PR. |
This was referenced Apr 27, 2018
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR can be referenced to discuss issues that need to be resolved to merge the expressions branch: expr_dev.
Dependencies: