Fix non-rigorous crossed bound results#3948
Conversation
bernalde
left a comment
bernalde
left a comment
There was a problem hiding this comment.
Blocking issue found in the LOA quadratic convexity classification. GitHub does not allow me to request changes on my own PR, so this is submitted as COMMENT, but I would not merge this until the blocking inline issue is addressed. Focused result-path tests pass locally; the broader changed-file pytest run is blocked by a local MCPP/libstdc++ loader mismatch in existing solver tests.
| repn = generate_standard_repn(expr, quadratic=True) | ||
| curvature = 0 | ||
| for coef, (v1, v2) in zip(repn.quadratic_coefs, repn.quadratic_vars): | ||
| if v1 is not v2: |
There was a problem hiding this comment.
Blocking: This treats every quadratic cross term as nonconvex/uncertified, but PSD/NSD quadratic forms can have off-diagonal terms. For example, x**2 + x*y + y**2 <= 4 is convex, and Pyomo already has GDP tests/model factories for that case in pyomo/gdp/tests/models.py and pyomo/gdp/tests/test_hull.py. With this detector, LOA marks those convex quadratic models as nonrigorous_dual_bound_possible, so any crossed-within-tolerance convergence returns feasible with an unbounded dual side instead of the certified optimal result users currently get. Please replace this diagonal-only sign check with a real PSD/NSD classification for quadratic forms, similar to the eigenvalue-based Q-matrix check used by the hull quadratic code, and add a regression test for a convex quadratic with an off-diagonal term.
There was a problem hiding this comment.
Addressed in 3fa0b4b. _quadratic_curvature() now builds the symmetric Q matrix and classifies PSD/NSD forms by eigenvalues, so convex off-diagonal terms are certified. Added test_loa_certifies_psd_quadratic_cross_terms for x**2 + x*y + y**2 <= 4 in pyomo/contrib/gdpopt/tests/test_gdpopt.py.
There was a problem hiding this comment.
Addressed in ce6b6d6. LOA now builds a symmetric Q matrix and classifies PSD/NSD forms by eigenvalues, including the no-NumPy fallback path. The regression coverage is in test_loa_certifies_psd_quadratic_cross_terms_without_numpy.
|
Addressed the open review thread. Commits pushed:
Main changes:
Tests run:
Comments intentionally not addressed:
Remaining risks or follow-up:
|
bernalde
left a comment
bernalde
left a comment
There was a problem hiding this comment.
I found one blocking issue. GitHub does not allow me to request changes on my own PR, so this is submitted as COMMENT, but I would not merge this until the blocking inline issue is addressed.
| if not repn.quadratic_coefs: | ||
| return 0 | ||
|
|
||
| if numpy_available: |
There was a problem hiding this comment.
Blocking: This makes certification of convex quadratic cross terms depend on optional NumPy. In the linux/3.10/slim CI job, NumPy is not installed, so this branch is skipped, _quadratic_curvature() falls through to the diagonal-only fallback, and test_loa_certifies_psd_quadratic_cross_terms fails because x**2 + x*y + y**2 <= 4 is still classified as potentially nonrigorous. This leaves the reviewed regression fixed only in optional-dependency environments. Please either implement the PSD/NSD check without requiring NumPy, or explicitly gate both the behavior and the test on NumPy availability; the former better preserves LOA's existing minimal-dependency behavior.
There was a problem hiding this comment.
Addressed in ce6b6d6. The diagonal-only fallback was replaced with a pure-Python Jacobi eigenvalue fallback when NumPy is unavailable, and the new no-NumPy test covers both a PSD cross-term form and an indefinite bilinear form.
|
Pushed Main changes:
Tests run:
Broader local check:
Comments intentionally not addressed:
Remaining risks / follow-up:
|
bernalde
left a comment
bernalde
left a comment
There was a problem hiding this comment.
Blocking issues: none found.
Nonblocking issues: none.
Questions: none.
Tests run and outcomes:
- PYTHONDONTWRITEBYTECODE=1 python -m pytest -q -p no:cacheprovider pyomo/contrib/gdpopt/tests/test_gdpopt.py::TestGDPoptUnit pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py::TestMindtPyGOAResults -> 16 passed, 2 skipped in 1.10s
- PYTHONDONTWRITEBYTECODE=1 python -m pytest -q -p no:cacheprovider pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py -> 29 passed in 1.51s
- PYTHONDONTWRITEBYTECODE=1 python -m pytest -q -p no:cacheprovider pyomo/contrib/gdpopt/tests/test_gdpopt.py -> 7 failed, 59 passed, 19 skipped, 3 deselected in 35.04s; all failures are local mcppInterface.so loader failures from GLIBCXX_3.4.32 not found, matching the existing local environment issue and not this PR's changed behavior
- black --check --diff pyomo/contrib/gdpopt/algorithm_base_class.py pyomo/contrib/gdpopt/loa.py pyomo/contrib/gdpopt/tests/test_gdpopt.py pyomo/contrib/mindtpy/algorithm_base_class.py pyomo/contrib/mindtpy/global_outer_approximation.py pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py -> passed, 6 files unchanged
- Additional no-NumPy smoke check: PSD cross-term classified certified, indefinite bilinear classified nonrigorous, mutable-Param PSD case classified certified
CI evidence inspected:
- linux/3.10/slim, linux/3.10/bare-env, linux/3.12/numpy2, lint/style-and-typos, docs, and several macOS/Linux jobs passed.
- The failed full Linux and win/3.10/pip jobs fail in GAMS solver tests with GAMS returncode=7, not in GDPopt or MindtPy. Jenkins and several matrix jobs are still pending.
I would not merge as-is until the required CI/Jenkins state is resolved, but I did not find PR-code changes that need to be addressed before merge.
1 participant
Summary
feasibleand report no certified dual bound instead of manufacturingoptimalwith equal bounds.Tests run
pytest -q pyomo/contrib/gdpopt/tests/test_gdpopt.py::TestGDPoptUnit::test_loa_crossed_bounds_are_not_reported_as_global_optimal pyomo/contrib/gdpopt/tests/test_gdpopt.py::TestGDPoptUnit::test_loa_nonpolynomial_model_may_have_nonrigorous_dual_bound pyomo/contrib/gdpopt/tests/test_gdpopt.py::TestGDPoptUnit::test_loa_distinguishes_basic_quadratic_convexity pyomo/contrib/gdpopt/tests/test_gdpopt.py::TestGDPoptUnit::test_gloa_crossed_bounds_preserve_certified_optimal_behavior pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py::TestMindtPyGOAResults::test_goa_crossed_bounds_are_not_reported_as_global_optimal pyomo/contrib/gdpopt/tests/test_gdpopt.py::TestGDPopt::test_logical_constraints_on_disjuncts_nonlinear_convex pyomo/contrib/gdpopt/tests/test_gdpopt.py::TestGDPopt::test_reverse_numeric_differentiation_in_LOA-> 7 passedpytest -q pyomo/contrib/gdpopt/tests/test_gdpopt.py::TestGDPoptUnit pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py-> 42 passed, 2 skippedpytest -q pyomo/contrib/gdpopt/tests/test_gdpopt.py-> 64 passed, 19 skipped, 3 deselectedpytest -q pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py-> 29 passedblack --check --diff pyomo/contrib/gdpopt/algorithm_base_class.py pyomo/contrib/gdpopt/loa.py pyomo/contrib/gdpopt/tests/test_gdpopt.py pyomo/contrib/mindtpy/algorithm_base_class.py pyomo/contrib/mindtpy/global_outer_approximation.py pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py-> passed; Black emitted its Python 3.13 vs 3.14 parser warning and reported all six files unchangedNotes
pytest -q pyomo/contrib/gdpopt/tests/test_gdpopt.py pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.pyhit local MC++ shared-library loading failures (GLIBCXX_3.4.32missing from the active libstdc++ for/home/bernalde/.pyomo/lib/mcppInterface.so); the same files passed when run separately as listed above.Closes #3947