Skip to content

Commit bbe8fba

Browse files
DLWoodruffclaude
andauthored
FWPH: forward linearize_* so the can't-linearize warning fires (#274) (#710)
* 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 90c3bec commit bbe8fba

3 files changed

Lines changed: 127 additions & 4 deletions

File tree

mpisppy/opt/fwph.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -931,16 +931,18 @@ def _options_checks_fw(self):
931931
# proximal terms (no binary variables allowed in FWPH QPs)
932932
if ('linearize_binary_proximal_terms' in self.options
933933
and self.options['linearize_binary_proximal_terms']):
934-
print('Warning: linearize_binary_proximal_terms cannot be used '
935-
'with the FWPH algorithm. Ignoring...')
934+
if self.cylinder_rank == 0:
935+
print('Warning: linearize_binary_proximal_terms cannot be used '
936+
'with the FWPH algorithm. Ignoring...')
936937
self.options['linearize_binary_proximal_terms'] = False
937938

938939
# 3b. Check that the user did not specify the linearization of all
939940
# proximal terms (FWPH QPs should be QPs)
940941
if ('linearize_proximal_terms' in self.options
941942
and self.options['linearize_proximal_terms']):
942-
print('Warning: linearize_proximal_terms cannot be used '
943-
'with the FWPH algorithm. Ignoring...')
943+
if self.cylinder_rank == 0:
944+
print('Warning: linearize_proximal_terms cannot be used '
945+
'with the FWPH algorithm. Ignoring...')
944946
self.options['linearize_proximal_terms'] = False
945947

946948
# 4. Provide a time limit of inf if the user did not specify

mpisppy/tests/test_config.py

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,5 +548,113 @@ def test_coeff_rho_args_added(self):
548548
self.assertIn("coeff_rho", cfg)
549549

550550

551+
class TestFWPHLinearizeForwarding(unittest.TestCase):
552+
"""The fwph_hub / fwph_spoke factories must forward
553+
cfg.linearize_proximal_terms and cfg.linearize_binary_proximal_terms
554+
into the options dict so FWPH._options_checks_fw can warn that
555+
FWPH cannot honor them (issue #274)."""
556+
557+
def _make_cfg(self, lin_prox=True, lin_bin=True):
558+
cfg = Config()
559+
cfg.popular_args()
560+
cfg.ph_args()
561+
cfg.fwph_args()
562+
cfg.two_sided_args()
563+
cfg.solver_name = "gurobi"
564+
cfg.linearize_proximal_terms = lin_prox
565+
cfg.linearize_binary_proximal_terms = lin_bin
566+
return cfg
567+
568+
def _beans(self, cfg):
569+
# placeholder values for the positional args of the factories;
570+
# nothing here is actually called because the factories just
571+
# stuff them into the returned dict.
572+
return dict(
573+
scenario_creator=lambda *a, **kw: None,
574+
scenario_denouement=None,
575+
all_scenario_names=["Scenario1"],
576+
)
577+
578+
def test_fwph_hub_forwards_linearize_options(self):
579+
import mpisppy.utils.cfg_vanilla as vanilla
580+
cfg = self._make_cfg(lin_prox=True, lin_bin=True)
581+
hub = vanilla.fwph_hub(cfg, **self._beans(cfg))
582+
opts = hub["opt_kwargs"]["options"]
583+
self.assertTrue(opts["linearize_proximal_terms"])
584+
self.assertTrue(opts["linearize_binary_proximal_terms"])
585+
586+
def test_fwph_spoke_forwards_linearize_options(self):
587+
import mpisppy.utils.cfg_vanilla as vanilla
588+
cfg = self._make_cfg(lin_prox=True, lin_bin=True)
589+
spoke = vanilla.fwph_spoke(cfg, **self._beans(cfg))
590+
opts = spoke["opt_kwargs"]["options"]
591+
self.assertTrue(opts["linearize_proximal_terms"])
592+
self.assertTrue(opts["linearize_binary_proximal_terms"])
593+
594+
def test_fwph_hub_forwards_false_when_off(self):
595+
import mpisppy.utils.cfg_vanilla as vanilla
596+
cfg = self._make_cfg(lin_prox=False, lin_bin=False)
597+
hub = vanilla.fwph_hub(cfg, **self._beans(cfg))
598+
opts = hub["opt_kwargs"]["options"]
599+
self.assertFalse(opts["linearize_proximal_terms"])
600+
self.assertFalse(opts["linearize_binary_proximal_terms"])
601+
602+
603+
class TestFWPHOptionsChecksWarnings(unittest.TestCase):
604+
"""FWPH._options_checks_fw must print a warning (rank 0 only) when
605+
linearize_proximal_terms or linearize_binary_proximal_terms is on,
606+
and must clear the flag so the QP solve proceeds."""
607+
608+
def _stub(self, lin_prox=False, lin_bin=False, rank=0):
609+
import types
610+
stub = types.SimpleNamespace()
611+
stub.cylinder_rank = rank
612+
stub.options = {
613+
"linearize_proximal_terms": lin_prox,
614+
"linearize_binary_proximal_terms": lin_bin,
615+
}
616+
stub.FW_options = {
617+
"FW_iter_limit": 1,
618+
"FW_weight": 0.0,
619+
"FW_conv_thresh": 1e-4,
620+
"solver_name": "gurobi",
621+
}
622+
return stub
623+
624+
def _run(self, stub):
625+
import io
626+
import contextlib
627+
from mpisppy.opt.fwph import FWPH
628+
buf = io.StringIO()
629+
with contextlib.redirect_stdout(buf):
630+
FWPH._options_checks_fw(stub)
631+
return buf.getvalue()
632+
633+
def test_warn_and_clear_linearize_proximal_on_rank0(self):
634+
stub = self._stub(lin_prox=True, rank=0)
635+
out = self._run(stub)
636+
self.assertIn("linearize_proximal_terms cannot be used", out)
637+
self.assertFalse(stub.options["linearize_proximal_terms"])
638+
639+
def test_warn_and_clear_linearize_binary_proximal_on_rank0(self):
640+
stub = self._stub(lin_bin=True, rank=0)
641+
out = self._run(stub)
642+
self.assertIn("linearize_binary_proximal_terms cannot be used", out)
643+
self.assertFalse(stub.options["linearize_binary_proximal_terms"])
644+
645+
def test_no_warning_on_nonzero_rank(self):
646+
stub = self._stub(lin_prox=True, lin_bin=True, rank=1)
647+
out = self._run(stub)
648+
self.assertEqual(out, "")
649+
# flag is still cleared regardless of rank
650+
self.assertFalse(stub.options["linearize_proximal_terms"])
651+
self.assertFalse(stub.options["linearize_binary_proximal_terms"])
652+
653+
def test_no_warning_when_flags_off(self):
654+
stub = self._stub(lin_prox=False, lin_bin=False, rank=0)
655+
out = self._run(stub)
656+
self.assertEqual(out, "")
657+
658+
551659
if __name__ == "__main__":
552660
unittest.main()

mpisppy/utils/cfg_vanilla.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,14 @@ def fwph_hub(cfg,
409409

410410
options.update(_fwph_options(cfg))
411411

412+
# Forward linearize_* options so FWPH._options_checks_fw can warn that
413+
# FWPH cannot honor them. Without this forwarding, the user sets
414+
# --linearize-proximal-terms, FWPH never sees it in self.options, and
415+
# there is no warning when FWPH proceeds with its (intended) quadratic
416+
# objective.
417+
options["linearize_proximal_terms"] = cfg.linearize_proximal_terms
418+
options["linearize_binary_proximal_terms"] = cfg.linearize_binary_proximal_terms
419+
412420
hub_dict = {
413421
"hub_class": FWPHHub,
414422
"hub_kwargs": {"options": {"rel_gap": cfg.rel_gap,
@@ -773,6 +781,11 @@ def fwph_spoke(
773781

774782
options.update(_fwph_options(cfg))
775783

784+
# Match fwph_hub: forward linearize_* so FWPH._options_checks_fw can
785+
# warn that FWPH cannot honor them. See the comment in fwph_hub.
786+
options["linearize_proximal_terms"] = cfg.linearize_proximal_terms
787+
options["linearize_binary_proximal_terms"] = cfg.linearize_binary_proximal_terms
788+
776789
fw_dict = {
777790
"spoke_class": FrankWolfeOuterBound,
778791
"opt_class": FWPH,

0 commit comments

Comments
 (0)