Internal/mindtpy short circuit base by bernalde · Pull Request #3907 · Pyomo/pyomo

bernalde · 2026-04-07T13:51:59Z

Fixes #3906 .

Summary/Motivation:

Based on a comment from @jsiirola in #3861 I went ahead and revisited how we are dealing with a problem that has no discrete variables. Now we check whether the MIP solver (eventually we need to change that nomenclature, but I will leave that to a future PR) can address the problem directly. This would be the case with quadratic programs (QPs) or quadratically constrained programs (QCPs). Although Gurobi can deal with NLPs now, usually it would try to solve them to global optimality, defeating the whole purpose of MindtPy, so we are still passing that down to the NLP (again, I need to change the naming here) solver.

This PR depends on #3861, so it is worth looking into it only after merging that

Changes proposed in this PR:

Modification in the 'shortcircuit' part of MindtPy and added tests

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:

I agree my contributions are submitted under the BSD license.
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.

- Fix stale Returns docstring in model_is_valid: LP, QP, QCP, or NLP - Defer working_obj fetch into the obj_degree is None fallback branch - Add Parameters section to _classify_short_circuit_problem docstring - Add comment in _mip_solver_supports_capability explaining that unknown APPSI solvers fall through conservatively to return False - Add test_short_circuit_mixed_degree_model_routes_to_nlp to cover the case where both has_quadratic_constraints and has_nonquadratic_constraints are True (model with quadratic and cubic constraints must route to NLP) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…lowup [codex] Refine MindtPy no-discrete short-circuit routing

jsiirola · 2026-04-10T15:07:30Z

@bernalde, @Toflamus: Note that this PR is currently blocked by merge conflicts introduced by #3861.

bernalde · 2026-04-10T17:03:08Z

@jsiirola Addressed in bernalde@888bf23.

I merged current main into internal/mindtpy-short-circuit-base, which clears the branch-blocking state noted in your comment. GitHub is now reporting this PR as mergeable again.

I also reran python -m pytest pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py on the updated branch.

codecov · 2026-04-10T18:06:12Z

Codecov Report

❌ Patch coverage is 99.09091% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.08%. Comparing base (ae432ba) to head (aea900a).

Files with missing lines	Patch %	Lines
pyomo/contrib/mindtpy/algorithm_base_class.py	99.09%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3907      +/-   ##
==========================================
+ Coverage   90.07%   90.08%   +0.01%     
==========================================
  Files         904      904              
  Lines      107003   107067      +64     
==========================================
+ Hits        96384    96454      +70     
+ Misses      10619    10613       -6

Flag	Coverage Δ
builders	`29.14% <4.54%> (-0.02%)`	⬇️
default	`86.41% <99.09%> (?)`
expensive	`35.57% <4.54%> (?)`
linux	`87.56% <99.09%> (-2.01%)`	⬇️
linux_other	`87.56% <99.09%> (+0.01%)`	⬆️
oldsolvers	`28.09% <4.54%> (-0.01%)`	⬇️
osx	`82.94% <99.09%> (+0.01%)`	⬆️
win	`86.00% <99.09%> (+0.01%)`	⬆️
win_other	`86.00% <99.09%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

bernalde · 2026-04-10T18:10:33Z

As far as I could see, none of the failed tests had to do with code introduced in this PR. We can restart the tests later to make sure everything is working

jsiirola

Overall, this looks pretty good. There is one case where woul will end up with an unexpected exception because of the order in which things are being processed (you are doing the short circuit handling before you have checked / normalized the objective).

bernalde · 2026-04-23T00:42:11Z

-                    'Using NLP solver %s to solve.' % config.nlp_solver
+            original_obj = next(
+                self.original_model.component_data_objects(ctype=Objective, active=True)
+            )


Short-circuit routing now depends on objective metadata before MindtPy has normalized the objective state. This path, together with set_up_solve_data(), still assumes there is exactly one active objective, so models with no objective will still raise unexpectedly and multiobjective models can take the first objective silently. Please validate or normalize the objective once before the short-circuit classification path and then use the cached degree directly here.

Addressed in ee55f3921. Objective normalization now happens once in set_up_solve_data(), the short-circuit path consumes the cached degree directly, multiobjective models raise early, and no-objective models use a temporary dummy objective that is cleaned up before returning.

bernalde · 2026-04-23T00:42:20Z

+    )
+
+
+class TestMindtPyShortCircuitRouting(unittest.TestCase):


These routing tests cover the LP, QP, QCP, and NLP branches well, but they do not exercise the objective-normalization edge cases that this short-circuit path now depends on. Please add regressions for both a no-objective model and a multiobjective model so the behavior stays aligned with process_objective() and set_up_solve_data().

Addressed in ee55f3921. I added test_short_circuit_model_with_no_objective_uses_temporary_dummy_objective and test_short_circuit_multiobjective_model_raises in pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py.

bernalde · 2026-04-23T00:51:18Z

Addressed the requested review changes in ee55f3921. The fix normalizes the objective before short-circuit routing, removes the redundant cached-degree re-check, adds regressions for missing-objective and multiobjective models, and keeps the current capability-based QP and QCP routing behavior in place for this PR. Tests run: python -m pytest pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py -q and python -m pytest pyomo/contrib/mindtpy/tests -q (52 passed, 32 skipped). For the broader QuadraticRepnVisitor follow-up, I opened #3920.

jsiirola

(pending passing tests)

mrmundt · 2026-05-05T20:12:24Z

+        appsi_capabilities = {
+            'appsi_cplex': {'quadratic_objective': True, 'quadratic_constraint': True},
+            'appsi_gurobi': {'quadratic_objective': True, 'quadratic_constraint': True},
+            'appsi_highs': {


Not a this-PR problem most likely, but it's probably a good idea for MindtPy to migrate away from supporting / relying on APPSI sometime in the future and instead move towards the contrib.solver interfaces.

Not addressed in this PR. This would change the solver-interface strategy beyond the continuous short-circuit fix, so I opened #3936 to track evaluating a MindtPy migration from APPSI-specific routing to contrib.solver interfaces.

mrmundt · 2026-05-05T20:13:21Z

-            self.best_solution_found = None
-            self.best_solution_found_time = None
-            self.initialize_mip_problem()
+                # TODO: if the MindtPy solver is defined once and called several times to solve models. The following two lines are necessary. It seems that the solver class will not be init every time call.


Nitpick - can you please break this comment onto multiple lines? It's really long.

Addressed in ca776cdee. I split the long TODO into multiple comment lines and kept the existing behavior unchanged: https://github.com/bernalde/pyomo/blob/ca776cdee086aac3419576ad3860d54a28bf7fbb/pyomo/contrib/mindtpy/algorithm_base_class.py#L3075.

emma58

Several comments below, but they're all picky or optional--nothing that needs to block the PR.

emma58 · 2026-05-06T01:38:50Z

+        if (
+            self.original_model.component(self._temporary_original_objective_name)
+            is not None
+        ):
+            self.original_model.del_component(self._temporary_original_objective_name)


del_component doesn't actually raise an error if you're deleting by name and the name doesn't exist. I only know because I just tried it out of curiosity... But for better or for worse, you don't need the if clause. In fact, m.del_component(None) is silent too, so you could just call it as long as you have the model, even if the name is None.

Addressed in ca776cdee. _cleanup_temporary_original_objective() now calls del_component() whenever original_model is available and then clears the stored temporary objective name: https://github.com/bernalde/pyomo/blob/ca776cdee086aac3419576ad3860d54a28bf7fbb/pyomo/contrib/mindtpy/algorithm_base_class.py#L290.

emma58 · 2026-05-06T01:44:45Z

+            setattr(model, dummy_name, Objective(expr=1))
+            return getattr(model, dummy_name), objective_count, dummy_name


What you have works fine, but just a style thing, you might do:

obj = Objective(expr=1) model.add_component(dummy_name, obj) return obj, objective_count, dummy_name

Addressed in ca776cdee. The dummy objective is now constructed first, added with model.add_component(...), and returned directly: https://github.com/bernalde/pyomo/blob/ca776cdee086aac3419576ad3860d54a28bf7fbb/pyomo/contrib/mindtpy/algorithm_base_class.py#L282.

emma58 · 2026-05-06T01:51:35Z

        return True

+    def _classify_short_circuit_problem(self, MindtPy, obj_degree):
+        """Classify a no-discrete model for short-circuit direct solves.


Suggested change

"""Classify a no-discrete model for short-circuit direct solves.

"""Classify a continuous model for short-circuit direct solves.

Addressed in ca776cdee. The docstring now says continuous model for the short-circuit direct-solve classifier: https://github.com/bernalde/pyomo/blob/ca776cdee086aac3419576ad3860d54a28bf7fbb/pyomo/contrib/mindtpy/algorithm_base_class.py#L443.

emma58 · 2026-05-06T01:53:27Z

+        """Classify a no-discrete model for short-circuit direct solves.
+
+        This classification is intentionally independent of
+        ``quadratic_strategy``. In the no-discrete short-circuit path we are


Suggested change

``quadratic_strategy``. In the no-discrete short-circuit path we are

``quadratic_strategy``. In the continuous-model short-circuit path we are

(Or something like that?)

Addressed in ca776cdee. The paragraph now uses continuous-model short-circuit path: https://github.com/bernalde/pyomo/blob/ca776cdee086aac3419576ad3860d54a28bf7fbb/pyomo/contrib/mindtpy/algorithm_base_class.py#L445.

emma58 · 2026-05-06T01:55:43Z

+                # TODO: revisit if/when the APPSI HiGHS wrapper adds QP support.
+                'quadratic_objective': False,
+                'quadratic_constraint': False,
+            },


Also not a this-PR problem, but there is quadratic support in the contrib.solver HiGHS interface now.

Not addressed in this PR. This belongs with the solver-interface migration rather than this behavior-preserving short-circuit change, so I opened #3936 to track evaluating contrib.solver routing and HiGHS quadratic support for MindtPy.

emma58 · 2026-05-06T02:02:10Z

            self.build_ordered_component_lists(model)
            self.add_cuts_components(model)

+    def _get_main_objective(self, model, create_dummy_objective=False, logger=None):


It looks like you always call this with create_dummy_objective=True. Do you need the argument?

Addressed in ca776cdee. I removed the unused create_dummy_objective argument from _get_main_objective() and updated both callers to use the simplified signature: https://github.com/bernalde/pyomo/blob/ca776cdee086aac3419576ad3860d54a28bf7fbb/pyomo/contrib/mindtpy/algorithm_base_class.py#L252, https://github.com/bernalde/pyomo/blob/ca776cdee086aac3419576ad3860d54a28bf7fbb/pyomo/contrib/mindtpy/algorithm_base_class.py#L874, https://github.com/bernalde/pyomo/blob/ca776cdee086aac3419576ad3860d54a28bf7fbb/pyomo/contrib/mindtpy/algorithm_base_class.py#L988.

emma58 · 2026-05-06T02:06:27Z

        """
        config = self.config
+        self.original_model = model
+        self._temporary_original_objective_name = None


This line looks unnecessary? I think it has to already be None, and even if not, _get_main_objective doesn't use it and you are setting it in line 1006 below.

Addressed in ca776cdee. I removed the redundant reset before _get_main_objective(); set_up_solve_data() now assigns _temporary_original_objective_name only from the returned dummy objective name: https://github.com/bernalde/pyomo/blob/ca776cdee086aac3419576ad3860d54a28bf7fbb/pyomo/contrib/mindtpy/algorithm_base_class.py#L987.

bernalde · 2026-05-06T21:36:40Z

For the top-level Codecov report, no separate coverage-only change was made. The report shows 98.24561% patch coverage, and the author revision in ca776cdee only addresses review cleanup without adding new behavior branches.

Local checks for this revision:
python -m pytest pyomo/contrib/mindtpy/tests/test_mindtpy_no_discrete.py -q passed with 28 tests.
python -m pytest pyomo/contrib/mindtpy/tests -q passed with 52 tests and 32 skipped.
python -m black --check --diff --fast pyomo/contrib/mindtpy/algorithm_base_class.py passed.
git diff --check passed.

bernalde · 2026-05-08T12:51:35Z

Could you please share the failure reason in Jenkins so I can fix this PR? Thanks!

jsiirola · 2026-05-08T14:08:10Z

Could you please share the failure reason in Jenkins so I can fix this PR? Thanks!

It's unrelated (it has to do with cuOpt and #3916)

bernalde and others added 6 commits April 6, 2026 18:35

Refine MindtPy short-circuit QP/QCP routing

685a264

Merge branch 'Pyomo:main' into internal/mindtpy-short-circuit-base

d1ac380

Address review feedback on short-circuit routing

a014410

Polish MindtPy short-circuit logging docs

da366fb

Merge pull request #1 from bernalde/mindtpy-short-circuit-routing-fol…

b9fa6fd

…lowup [codex] Refine MindtPy no-discrete short-circuit routing

Merge upstream/main into internal/mindtpy-short-circuit-base

888bf23

jsiirola added the AT: PRE-TEST INSPECTED label Apr 10, 2026

pyomo-autotest removed the AT: PRE-TEST INSPECTED label Apr 10, 2026

Merge branch 'main' into internal/mindtpy-short-circuit-base

72f7c43

jsiirola added the AT: PRE-TEST INSPECTED label Apr 13, 2026

pyomo-autotest removed the AT: PRE-TEST INSPECTED label Apr 13, 2026

jsiirola requested changes Apr 22, 2026

View reviewed changes

Comment thread pyomo/contrib/mindtpy/algorithm_base_class.py

Comment thread pyomo/contrib/mindtpy/algorithm_base_class.py Outdated

Comment thread pyomo/contrib/mindtpy/algorithm_base_class.py

Comment thread pyomo/contrib/mindtpy/algorithm_base_class.py

bernalde commented Apr 23, 2026

View reviewed changes

Normalize MindtPy objective handling before short-circuit routing

ee55f39

bernalde added 2 commits April 22, 2026 21:25

Format MindtPy objective normalization helpers

29145ba

Merge branch 'main' into internal/mindtpy-short-circuit-base

93fd309

jsiirola added the AT: PRE-TEST INSPECTED label Apr 23, 2026

jsiirola approved these changes Apr 23, 2026

View reviewed changes

pyomo-autotest removed the AT: PRE-TEST INSPECTED label Apr 23, 2026

blnicho requested a review from emma58 April 28, 2026 18:49

jsiirola added the PR:need_additional_review label Apr 28, 2026

mrmundt reviewed May 5, 2026

View reviewed changes

emma58 approved these changes May 6, 2026

View reviewed changes

Address MindtPy review nits

ca776cd

bernalde mentioned this pull request May 6, 2026

MindtPy follow-up: evaluate contrib.solver migration for direct continuous solves #3936

Open

jsiirola added AT: PRE-TEST INSPECTED and removed PR:need_additional_review labels May 7, 2026

pyomo-autotest removed the AT: PRE-TEST INSPECTED label May 7, 2026

Merge branch 'main' into internal/mindtpy-short-circuit-base

aea900a

jsiirola added the AT: PRE-TEST INSPECTED label May 8, 2026

pyomo-autotest removed the AT: PRE-TEST INSPECTED label May 8, 2026

jsiirola merged commit 3b18254 into Pyomo:main May 8, 2026
64 of 65 checks passed

		setattr(model, dummy_name, Objective(expr=1))
		return getattr(model, dummy_name), objective_count, dummy_name

	"""Classify a no-discrete model for short-circuit direct solves.
	"""Classify a continuous model for short-circuit direct solves.

	``quadratic_strategy``. In the no-discrete short-circuit path we are
	``quadratic_strategy``. In the continuous-model short-circuit path we are

Conversation

bernalde commented Apr 7, 2026

Fixes #3906 .

Summary/Motivation:

Changes proposed in this PR:

Legal Acknowledgement

Uh oh!

jsiirola commented Apr 10, 2026

Uh oh!

bernalde commented Apr 10, 2026

Uh oh!

codecov Bot commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bernalde commented Apr 10, 2026

Uh oh!

jsiirola left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bernalde commented Apr 23, 2026

Uh oh!

jsiirola left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emma58 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bernalde commented May 6, 2026

Uh oh!

bernalde commented May 8, 2026

Uh oh!

codecov Bot commented Apr 10, 2026 •

edited

Loading