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

Identify squared linear sums as quadratic with generate_standard_repn #1493

Merged
merged 8 commits into from
Jun 17, 2020

Conversation

johwiebe
Copy link
Contributor

@johwiebe johwiebe commented Jun 10, 2020

Fixes #1287

Summary/Motivation:

generate_standard_repn should recognize the square of a linear sum (x_1 + x_2 + ... + x_n)**2 as a quadratic when quadratic=True. It currently returns a nonlinear expression if the linear sum has more than one term. This leads, e.g., to issues when trying to solve a model with a quadratic constraint of this form with Gurobi.

Note that there is a test case which asks for this behavior for expression e = (m.a + m.b)**2:

rep = generate_standard_repn(e, compute_values=False, quadratic=True)
#
self.assertFalse( rep.is_fixed() )
self.assertEqual( rep.polynomial_degree(), None )
self.assertFalse( rep.is_constant() )
self.assertFalse( rep.is_linear() )
self.assertFalse( rep.is_quadratic() )
self.assertTrue( rep.is_nonlinear() )
#
self.assertTrue(len(rep.linear_vars) == 0)
self.assertTrue(len(rep.linear_coefs) == 0)
self.assertTrue(len(rep.quadratic_vars) == 0)
self.assertTrue(len(rep.quadratic_coefs) == 0)
self.assertFalse(rep.nonlinear_expr is None)
self.assertTrue(len(rep.nonlinear_vars) == 2)
baseline = { }
self.assertEqual(baseline, repn_to_dict(rep))

Am I missing a reason why this behavior might be wanted?

Changes proposed in this PR:

  • _collect_pow returns a quadratic expression for any PowExpression with linear base and exponent 2. The underlying logic is:
    (sum_i x_i)**2 = sum_i sum_j x_i*x_j = sum_i x_i**2 + 2*sum_i sum_{j<i} x_i*x_j
  • Modifies a test case for expression (m.a + m.b)**2 which currently expects generate_standard_repn to return a nonlinear expression.
  • Adds a test case for expression (linear sum)**2 with more than two variables.

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.

johwiebe added 5 commits June 10, 2020 09:50
generate_standard_repn applied to (x+y)**2 now returns a quadratic
expression.
Standard repn for this should be m.a*m.a + 2*m.a*m.b + m.b*m.b which is
quadratic.
The square of a linear sum should be quadratic.
@codecov
Copy link

codecov bot commented Jun 11, 2020

Codecov Report

Merging #1493 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1493      +/-   ##
==========================================
+ Coverage   72.35%   72.38%   +0.02%     
==========================================
  Files         608      608              
  Lines       85595    85642      +47     
==========================================
+ Hits        61933    61991      +58     
+ Misses      23662    23651      -11     
Impacted Files Coverage Δ
pyomo/repn/standard_repn.py 92.97% <100.00%> (+0.06%) ⬆️
pyomo/pysp/scenariotree/tree_structure.py 83.57% <0.00%> (-0.22%) ⬇️
pyomo/util/check_units.py 94.56% <0.00%> (+2.40%) ⬆️
pyomo/solvers/plugins/solvers/BARON.py 81.53% <0.00%> (+2.43%) ⬆️
pyomo/contrib/gdpopt/nlp_solve.py 74.91% <0.00%> (+2.60%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e021c09...529766f. Read the comment docs.

Copy link
Member

@jsiirola jsiirola left a comment

Choose a reason for hiding this comment

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

I think this is OK to merge. The tests highlight a nondeterminism in the internal Results data structures, but resolving that is beyond the scope of this PR and I believe should be dealt with elsewhere (see #1073 and the solver-rewrite).

Comment on lines 751 to 752
for key2, coef2 in res.linear.items():
ans.quadratic[key1,key2] = 2*multiplier*coef1*coef2
Copy link
Member

Choose a reason for hiding this comment

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

Note that this is inherently nondeterministic. Unfortunately, this is a flaw in the current Results data structures. It will be resolved n an upcoming rewrite of generate_standard_repn (see #1073 and the draft implementation on the solver-rewrite branch)

Copy link
Member

Choose a reason for hiding this comment

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

I stand corrected. @carldlaird pointed out to me that this can be made deterministic relatively easily. Expect a PR from him that will resolve this and then we can merge.

@blnicho blnicho merged commit f5b0392 into Pyomo:master Jun 17, 2020
@johwiebe johwiebe deleted the squared_lin_exp branch June 17, 2020 08:27
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.

Quadratic term not identified by generate_standard_repn
6 participants