FWPH: forward linearize_* so the can't-linearize warning fires (#274)#710
Merged
Conversation
…#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 Pyomo#274 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #710 +/- ##
==========================================
+ Coverage 71.19% 71.28% +0.09%
==========================================
Files 154 154
Lines 19432 19438 +6
==========================================
+ Hits 13835 13857 +22
+ Misses 5597 5581 -16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fwph_hub/fwph_spokefactories did not forwardcfg.linearize_proximal_termsorcfg.linearize_binary_proximal_termsinto the options dict handed to FWPH, soFWPH._options_checks_fwnever saw the flag and the user got no warning when they combined--linearize-proximal-termswith--fwph. With an LP-only solver (e.g.glpk) optimization silently proceeded and then failed in the QP solve, exactly as Matthew-Signorotti reported.cfg_vanilla.fwph_hubandcfg_vanilla.fwph_spokenow explicitly setoptions["linearize_proximal_terms"]andoptions["linearize_binary_proximal_terms"]after the_fwph_optionsmerge, mirroring the existing pattern inph_hub.opt/fwph.py _options_checks_fw: gate the two warning prints onself.cylinder_rank == 0so HPC runs don't emit N copies of the same message.mpisppy/tests/test_config.pycover both the cfg_vanilla forwarding and the rank-0 warning paths inFWPH._options_checks_fw.Note: bknueven's follow-up suggestion (FWPH should accept separate
mip_solver_name/qp_solver_nameso you can pair glpk/cbc with ipopt) is tracked as a separate enhancement in #712.Test plan
ruff checkclean on changed filesmpiexec -np 2 ... --fwph --linearize-proximal-terms --solver-name glpknow prints the expected "linearize_proximal_terms cannot be used with the FWPH algorithm" warning before the (expected) glpk-cannot-do-QPs failurepytest mpisppy/tests/test_config.py)🤖 Generated with Claude Code