From 12f1d19759b174fca917b5cfbe8c8c7684af819e Mon Sep 17 00:00:00 2001 From: Dave Woodruff Date: Sun, 17 May 2026 15:17:42 -0700 Subject: [PATCH 1/2] FWPH: forward linearize_* so the can't-linearize warning fires (#274) The vanilla fwph_hub / fwph_spoke factories did not forward cfg.linearize_proximal_terms or cfg.linearize_binary_proximal_terms into the options dict handed to FWPH. _options_checks_fw therefore never saw the flag, so the user got no warning when they combined --linearize-proximal-terms with --fwph. With an LP-only solver (e.g. glpk), optimization then silently proceeded and ultimately failed in the QP solve. - cfg_vanilla.fwph_hub and cfg_vanilla.fwph_spoke: explicitly set options["linearize_proximal_terms"] and options["linearize_binary_proximal_terms"] after the _fwph_options merge, mirroring the existing pattern in ph_hub. - opt/fwph.py _options_checks_fw: gate the two warning prints on self.cylinder_rank == 0 so HPC runs don't emit N copies of the same message. Fixes #274 Co-Authored-By: Claude Opus 4.7 (1M context) --- mpisppy/opt/fwph.py | 10 ++++++---- mpisppy/utils/cfg_vanilla.py | 13 +++++++++++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/mpisppy/opt/fwph.py b/mpisppy/opt/fwph.py index fdf447315..90208d28f 100644 --- a/mpisppy/opt/fwph.py +++ b/mpisppy/opt/fwph.py @@ -931,16 +931,18 @@ def _options_checks_fw(self): # proximal terms (no binary variables allowed in FWPH QPs) if ('linearize_binary_proximal_terms' in self.options and self.options['linearize_binary_proximal_terms']): - print('Warning: linearize_binary_proximal_terms cannot be used ' - 'with the FWPH algorithm. Ignoring...') + if self.cylinder_rank == 0: + print('Warning: linearize_binary_proximal_terms cannot be used ' + 'with the FWPH algorithm. Ignoring...') self.options['linearize_binary_proximal_terms'] = False # 3b. Check that the user did not specify the linearization of all # proximal terms (FWPH QPs should be QPs) if ('linearize_proximal_terms' in self.options and self.options['linearize_proximal_terms']): - print('Warning: linearize_proximal_terms cannot be used ' - 'with the FWPH algorithm. Ignoring...') + if self.cylinder_rank == 0: + print('Warning: linearize_proximal_terms cannot be used ' + 'with the FWPH algorithm. Ignoring...') self.options['linearize_proximal_terms'] = False # 4. Provide a time limit of inf if the user did not specify diff --git a/mpisppy/utils/cfg_vanilla.py b/mpisppy/utils/cfg_vanilla.py index 4cea09835..5e15f380e 100644 --- a/mpisppy/utils/cfg_vanilla.py +++ b/mpisppy/utils/cfg_vanilla.py @@ -409,6 +409,14 @@ def fwph_hub(cfg, options.update(_fwph_options(cfg)) + # Forward linearize_* options so FWPH._options_checks_fw can warn that + # FWPH cannot honor them. Without this forwarding, the user sets + # --linearize-proximal-terms, FWPH never sees it in self.options, and + # there is no warning when FWPH proceeds with its (intended) quadratic + # objective. + options["linearize_proximal_terms"] = cfg.linearize_proximal_terms + options["linearize_binary_proximal_terms"] = cfg.linearize_binary_proximal_terms + hub_dict = { "hub_class": FWPHHub, "hub_kwargs": {"options": {"rel_gap": cfg.rel_gap, @@ -773,6 +781,11 @@ def fwph_spoke( options.update(_fwph_options(cfg)) + # Match fwph_hub: forward linearize_* so FWPH._options_checks_fw can + # warn that FWPH cannot honor them. See the comment in fwph_hub. + options["linearize_proximal_terms"] = cfg.linearize_proximal_terms + options["linearize_binary_proximal_terms"] = cfg.linearize_binary_proximal_terms + fw_dict = { "spoke_class": FrankWolfeOuterBound, "opt_class": FWPH, From 9187dde6a9146868c9a1bf59511ce09f0a0c7168 Mon Sep 17 00:00:00 2001 From: Dave Woodruff Date: Sun, 17 May 2026 16:09:31 -0700 Subject: [PATCH 2/2] test: cover FWPH linearize_* forwarding and warning paths Adds two test classes to test_config.py: - TestFWPHLinearizeForwarding asserts cfg_vanilla.fwph_hub and cfg_vanilla.fwph_spoke copy cfg.linearize_proximal_terms and cfg.linearize_binary_proximal_terms into the options dict (the forwarding lines added in this PR). - TestFWPHOptionsChecksWarnings drives FWPH._options_checks_fw with a stub and verifies the rank-0 warning prints fire (and stay silent on non-zero ranks) and that the offending flags are cleared. Covers the patch lines codecov flagged on PR 710. Co-Authored-By: Claude Opus 4.7 (1M context) --- mpisppy/tests/test_config.py | 108 +++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/mpisppy/tests/test_config.py b/mpisppy/tests/test_config.py index 97c197858..707cfb43a 100644 --- a/mpisppy/tests/test_config.py +++ b/mpisppy/tests/test_config.py @@ -548,5 +548,113 @@ def test_coeff_rho_args_added(self): self.assertIn("coeff_rho", cfg) +class TestFWPHLinearizeForwarding(unittest.TestCase): + """The fwph_hub / fwph_spoke factories must forward + cfg.linearize_proximal_terms and cfg.linearize_binary_proximal_terms + into the options dict so FWPH._options_checks_fw can warn that + FWPH cannot honor them (issue #274).""" + + def _make_cfg(self, lin_prox=True, lin_bin=True): + cfg = Config() + cfg.popular_args() + cfg.ph_args() + cfg.fwph_args() + cfg.two_sided_args() + cfg.solver_name = "gurobi" + cfg.linearize_proximal_terms = lin_prox + cfg.linearize_binary_proximal_terms = lin_bin + return cfg + + def _beans(self, cfg): + # placeholder values for the positional args of the factories; + # nothing here is actually called because the factories just + # stuff them into the returned dict. + return dict( + scenario_creator=lambda *a, **kw: None, + scenario_denouement=None, + all_scenario_names=["Scenario1"], + ) + + def test_fwph_hub_forwards_linearize_options(self): + import mpisppy.utils.cfg_vanilla as vanilla + cfg = self._make_cfg(lin_prox=True, lin_bin=True) + hub = vanilla.fwph_hub(cfg, **self._beans(cfg)) + opts = hub["opt_kwargs"]["options"] + self.assertTrue(opts["linearize_proximal_terms"]) + self.assertTrue(opts["linearize_binary_proximal_terms"]) + + def test_fwph_spoke_forwards_linearize_options(self): + import mpisppy.utils.cfg_vanilla as vanilla + cfg = self._make_cfg(lin_prox=True, lin_bin=True) + spoke = vanilla.fwph_spoke(cfg, **self._beans(cfg)) + opts = spoke["opt_kwargs"]["options"] + self.assertTrue(opts["linearize_proximal_terms"]) + self.assertTrue(opts["linearize_binary_proximal_terms"]) + + def test_fwph_hub_forwards_false_when_off(self): + import mpisppy.utils.cfg_vanilla as vanilla + cfg = self._make_cfg(lin_prox=False, lin_bin=False) + hub = vanilla.fwph_hub(cfg, **self._beans(cfg)) + opts = hub["opt_kwargs"]["options"] + self.assertFalse(opts["linearize_proximal_terms"]) + self.assertFalse(opts["linearize_binary_proximal_terms"]) + + +class TestFWPHOptionsChecksWarnings(unittest.TestCase): + """FWPH._options_checks_fw must print a warning (rank 0 only) when + linearize_proximal_terms or linearize_binary_proximal_terms is on, + and must clear the flag so the QP solve proceeds.""" + + def _stub(self, lin_prox=False, lin_bin=False, rank=0): + import types + stub = types.SimpleNamespace() + stub.cylinder_rank = rank + stub.options = { + "linearize_proximal_terms": lin_prox, + "linearize_binary_proximal_terms": lin_bin, + } + stub.FW_options = { + "FW_iter_limit": 1, + "FW_weight": 0.0, + "FW_conv_thresh": 1e-4, + "solver_name": "gurobi", + } + return stub + + def _run(self, stub): + import io + import contextlib + from mpisppy.opt.fwph import FWPH + buf = io.StringIO() + with contextlib.redirect_stdout(buf): + FWPH._options_checks_fw(stub) + return buf.getvalue() + + def test_warn_and_clear_linearize_proximal_on_rank0(self): + stub = self._stub(lin_prox=True, rank=0) + out = self._run(stub) + self.assertIn("linearize_proximal_terms cannot be used", out) + self.assertFalse(stub.options["linearize_proximal_terms"]) + + def test_warn_and_clear_linearize_binary_proximal_on_rank0(self): + stub = self._stub(lin_bin=True, rank=0) + out = self._run(stub) + self.assertIn("linearize_binary_proximal_terms cannot be used", out) + self.assertFalse(stub.options["linearize_binary_proximal_terms"]) + + def test_no_warning_on_nonzero_rank(self): + stub = self._stub(lin_prox=True, lin_bin=True, rank=1) + out = self._run(stub) + self.assertEqual(out, "") + # flag is still cleared regardless of rank + self.assertFalse(stub.options["linearize_proximal_terms"]) + self.assertFalse(stub.options["linearize_binary_proximal_terms"]) + + def test_no_warning_when_flags_off(self): + stub = self._stub(lin_prox=False, lin_bin=False, rank=0) + out = self._run(stub) + self.assertEqual(out, "") + + if __name__ == "__main__": unittest.main()