-
Notifications
You must be signed in to change notification settings - Fork 45
PHDualSpoke #535
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PHDualSpoke #535
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR renames the “PH nonant” feature to “Primal PH,” adds a new Dual PH spoke/configuration, and updates all related code paths, documentation, and examples to support both Primal and Dual PH.
- Renamed
ph_nonanttoprimal_phin config, hubs, and cylinder dispatch. - Introduced
dual_ph_argsand aDualPHSpokeimplementation with unified rho handling. - Updated docs (
spokes.rst,hubs.rst) and examples to demonstrate Primal and Dual PH usage.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| mpisppy/utils/config.py | Renamed ph_nonant_args → primal_ph_args, added dual_ph_args. |
| mpisppy/utils/cfg_vanilla.py | Updated imports and hub/spoke factory functions for Primal & Dual PH. |
| mpisppy/generic_cylinders.py | Hooked in new primal_ph_* and dual_ph logic in decomposition. |
| mpisppy/cylinders/relaxed_ph_spoke.py | Refactored to extend shared _DualPHSpokeBase logic. |
| mpisppy/cylinders/hub.py | Renamed PHNonantHub → PrimalPHHub and adjusted PHHub base. |
| mpisppy/cylinders/dual_ph_spoke.py | Added new Dual PH spoke classes (_DualPHSpokeBase, DualPHSpoke). |
| examples/run_all.py | Added example MPI calls for Primal and Dual PH spokes. |
| doc/src/spokes.rst | Documented “Relaxed PH” and new “Dual PH” options. |
| doc/src/hubs.rst | Updated hub docs to reference “Primal PH” instead of “PHNonant.” |
Comments suppressed due to low confidence (2)
mpisppy/utils/config.py:766
- [nitpick] Consider updating this description to an imperative and consistent style, e.g., 'Enable Dual PH spoke'.
description="have a dual PH spoke",
mpisppy/utils/config.py:763
- There are no tests covering the new
dual_ph_argsconfiguration or the Dual PH spoke; consider adding unit or integration tests for this feature.
def dual_ph_args(self):
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
DLWoodruff
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like everything about this PR, except the names "DualPH" and "PrimalPH". I prefer "PHDual" and "PHPrimal" for reasons related to the existence of "Dual Simplex" that I can describe during the zoom meeting, if necessary.
No description provided.