Skip to content

Add TestExactHullQuadratic tests and extract models into models.py#5

Merged
sergey-gusev94 merged 4 commits intoex_hullfrom
copilot/sub-pr-2-another-one
Mar 8, 2026
Merged

Add TestExactHullQuadratic tests and extract models into models.py#5
sergey-gusev94 merged 4 commits intoex_hullfrom
copilot/sub-pr-2-another-one

Conversation

Copy link
Copy Markdown

Copilot AI commented Mar 8, 2026

Adds a TestExactHullQuadratic test suite for the exact_hull_quadratic option of the hull transformation, and places all required test models in pyomo/gdp/tests/models.py following the convention used throughout the rest of the GDP test suite.

pyomo/gdp/tests/models.py

Eight new factory functions added at the end of the file:

Function Constraint on d1
makeTwoTermDisj_ConvexQuadUB() x**2 + y**2 <= 4
makeTwoTermDisj_ConvexQuadLB() -x**2 - y**2 >= -4
makeTwoTermDisj_NonconvexQuad() x**2 - y**2 <= 1
makeTwoTermDisj_QuadEquality() x**2 + y**2 == 4
makeTwoTermDisj_QuadRange() 1 <= x**2 + y**2 <= 4
makeTwoTermDisj_NearPSDQuad() x**2 - 1e-11*y**2 <= 4
makeTwoTermDisj_ConvexQuad_LinearTerms() x**2 + 3*x + 2 <= 0
makeTwoTermDisj_NonconvexQuad_LinearTerms() x**2 - y**2 + 3*x - 2 <= 0

pyomo/gdp/tests/test_hull.py

New TestExactHullQuadratic class with 11 tests covering:

  • Convex quadratic upper-bound → conic reformulation (PSD Q)
  • Convex quadratic lower-bound → conic reformulation via negation (NSD Q)
  • Non-convex quadratic → general exact hull (indefinite Q)
  • Equality constraint → always uses general exact hull
  • Range constraint → mixed conic + general reformulations
  • NumPy unavailable guard (runs in all environments, no skip decorator)
  • eigenvalue_tolerance permissive (1e-9, non-default) → treats near-PSD as PSD
  • eigenvalue_tolerance strict (1e-12) → rejects near-PSD matrix
  • get_transformed_constraints mapping preserved
  • Linear and constant terms in conic reformulation
  • Linear and constant terms in general exact hull

The class-level @skipUnless(numpy_available) has been replaced with per-test decorators on each test that genuinely requires NumPy. test_numpy_unavailable_raises_error is intentionally left undecorated so it validates the error-path in environments where NumPy is absent.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] WIP address feedback on hull reformulations and implement tests Add comprehensive tests for exact_hull_quadratic hull reformulation Mar 8, 2026
@sergey-gusev94
Copy link
Copy Markdown
Owner

Create models that you need in sepearate model file, similar how it is done for other tests

Copilot AI changed the title Add comprehensive tests for exact_hull_quadratic hull reformulation Extract exact hull quadratic test models into models.py Mar 8, 2026
@sergey-gusev94 sergey-gusev94 requested a review from Copilot March 8, 2026 05:55
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR centralizes model construction for the hull transformation’s exact_hull_quadratic test coverage by adding dedicated factory functions in pyomo/gdp/tests/models.py and using them from pyomo/gdp/tests/test_hull.py for a new/updated TestExactHullQuadratic test suite.

Changes:

  • Added eight makeTwoTermDisj_* model factory functions for quadratic exact-hull testing to pyomo/gdp/tests/models.py.
  • Added TestExactHullQuadratic tests in pyomo/gdp/tests/test_hull.py that construct models via models.* factories and validate conic/general exact-hull reformulations and mappings.
  • Introduced NumPy gating and a mocked “NumPy unavailable” guard test in the hull test suite.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
pyomo/gdp/tests/test_hull.py Adds a NumPy-gated TestExactHullQuadratic suite exercising exact-hull quadratic behavior, including mappings and eigenvalue tolerance behavior.
pyomo/gdp/tests/models.py Adds reusable model factory functions for the quadratic exact-hull tests to align with existing GDP test-suite model patterns.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pyomo/gdp/tests/test_hull.py Outdated
Comment on lines +2898 to +2899
@unittest.skipUnless(numpy_available, "NumPy is not available")
class TestExactHullQuadratic(unittest.TestCase):
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

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

The class-level @skipUnless(numpy_available) prevents the "numpy unavailable" behavior from being tested in an environment where NumPy is actually missing. In particular, test_numpy_unavailable_raises_error will be skipped when numpy_available is False, even though it is meant to validate that code path. Consider moving that test outside the skipped class (or applying the skip decorator only to tests that truly require NumPy), so that the error path is covered in minimal environments.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback
Apply the skip decorator only to tests that truly require NumPy

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done in commit 40c0077. Removed the class-level @skipUnless(numpy_available) decorator and added individual @skipUnless(numpy_available, "NumPy is not available") decorators to each of the 10 tests that actually require NumPy. test_numpy_unavailable_raises_error is left undecorated so it runs in all environments, including ones where NumPy is absent. Also updated test_eigenvalue_tolerance_permissive to use eigenvalue_tolerance=1e-9 (a clearly non-default permissive value) instead of 1e-10 (which happened to match the default).

Comment thread pyomo/gdp/tests/test_hull.py Outdated
Comment on lines +2898 to +2901
@unittest.skipUnless(numpy_available, "NumPy is not available")
class TestExactHullQuadratic(unittest.TestCase):
"""Tests for the ``exact_hull_quadratic`` option of the hull transformation.

Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

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

PR description says this change only extracts existing inline models and that no test logic changed, but this diff introduces an entirely new TestExactHullQuadratic test class (and new imports) in test_hull.py. Please either update the PR description/title to reflect that new tests are being added, or (if these tests existed before) ensure the diff includes the corresponding removals/moves so reviewers can confirm it's a pure refactor.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback
update the PR description/title

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Updated the PR title to "Add TestExactHullQuadratic tests with models extracted to models.py" and rewrote the description to accurately reflect that this PR adds a new 11-test TestExactHullQuadratic class (not a pure refactor), along with the eight model factory functions in models.py.

Comment thread pyomo/gdp/tests/test_hull.py Outdated
Comment on lines +3225 to +3232
(-1e-11). With the default tolerance of 1e-10 the eigenvalue is
within the tolerance band and Q is treated as PSD → conic
reformulation.
"""
m = models.makeTwoTermDisj_NearPSDQuad()

m_perm = self.hull.create_using(
m, exact_hull_quadratic=True, eigenvalue_tolerance=1e-10
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

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

The docstring describes this as using a "larger" eigenvalue_tolerance than default, but the test passes 1e-10, which matches gdp.hull's current default eigenvalue_tolerance. Either adjust the wording to indicate it's the default tolerance, or change the test to use a non-default permissive value so the test actually demonstrates the configuration effect.

Suggested change
(-1e-11). With the default tolerance of 1e-10 the eigenvalue is
within the tolerance band and Q is treated as PSDconic
reformulation.
"""
m = models.makeTwoTermDisj_NearPSDQuad()
m_perm = self.hull.create_using(
m, exact_hull_quadratic=True, eigenvalue_tolerance=1e-10
(-1e-11). The default eigenvalue_tolerance in gdp.hull is 1e-10;
here we use a larger, more permissive tolerance (1e-9) so the
eigenvalue is well within the tolerance band and Q is treated as
PSDconic reformulation.
"""
m = models.makeTwoTermDisj_NearPSDQuad()
m_perm = self.hull.create_using(
m, exact_hull_quadratic=True, eigenvalue_tolerance=1e-9

Copilot uses AI. Check for mistakes.
…envalue_tolerance in permissive test

Co-authored-by: sergey-gusev94 <[email protected]>
Copilot AI changed the title Extract exact hull quadratic test models into models.py Add TestExactHullQuadratic tests and extract models into models.py Mar 8, 2026
@sergey-gusev94 sergey-gusev94 requested a review from Copilot March 8, 2026 06:10
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sergey-gusev94 sergey-gusev94 marked this pull request as ready for review March 8, 2026 06:16
@sergey-gusev94 sergey-gusev94 merged commit 0c9bf78 into ex_hull Mar 8, 2026
5 checks passed
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