Skip to content

Conversation

@bknueven
Copy link
Collaborator

No description provided.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds helper utilities to define/apply more granular (per-spoke) solver configuration, including per-iteration mipgap settings, and wires these into several spokes.

Changes:

  • Refactors solver spec config creation into add_solver_specs() and new add_mipgap_specs() helpers.
  • Introduces apply_solver_specs() to apply per-spoke solver name/options/mipgap overrides when building spoke dictionaries.
  • Updates multiple spokes to use the shared helpers instead of inline mipgap wiring.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
mpisppy/utils/config.py Adds reusable config helpers for solver + mipgap specs and applies them across spokes.
mpisppy/utils/cfg_vanilla.py Adds apply_solver_specs() and simplifies _hasit (currently with a behavior-changing bug).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Collaborator

@DLWoodruff DLWoodruff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a big improvement.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

mpisppy/utils/solver_spec.py:73

  • If no solver name is found and name_required=False, this function will fall through and return solver_name/solver_options without them ever being initialized, leading to an UnboundLocalError. Initialize them before the loop (e.g., to None/{}) and return those defaults when no match is found.
            solver_options = sputils.option_string_to_dict(ostr)
            break
    else:
        if name_required:
            # Leaving in underscores even though it might confuse command line users

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@bknueven
Copy link
Collaborator Author

This is a big improvement.

Thanks, although I'm having trouble resolving some issues in the confidence interval code. If I don't get back to this, it would be a good topic for a future call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants