Skip to content

Conversation

@cfrontin
Copy link
Collaborator

@cfrontin cfrontin commented Aug 22, 2025

Discovered changes to COBYLA implementation across scipy 1.15 and 1.16 that necessitate opt_settings management, this allows that from Ard.

@cfrontin cfrontin requested a review from Copilot August 22, 2025 22:12
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

This PR improves the OpenMDAO (OM) driver interface by adding support for configuring optimization settings through the driver configuration. The change allows users to specify opt_settings alongside regular driver options, providing more granular control over optimization parameters.

  • Adds handling for opt_settings within driver configuration options
  • Maintains backward compatibility with existing driver option configuration

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

if "options" in analysis_options["driver"]:
for option, value in analysis_options["driver"]["options"].items():
prob.driver.options[option] = value
if option == "opt_settings":
Copy link

Copilot AI Aug 22, 2025

Choose a reason for hiding this comment

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

Using a string literal comparison for special option handling creates a magic string. Consider defining a constant like OPT_SETTINGS_KEY = "opt_settings" to make this more maintainable and reduce the risk of typos.

Suggested change
if option == "opt_settings":
if option == OPT_SETTINGS_KEY:

Copilot uses AI. Check for mistakes.
for opt_setting in value:
prob.driver.opt_settings[opt_setting] = value[opt_setting]
else:
prob.driver.options[option] = value
Copy link

Copilot AI Aug 22, 2025

Choose a reason for hiding this comment

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

The variable name opt_setting suggests a single setting, but it's being used as a key to access value[opt_setting]. This should be prob.driver.opt_settings[opt_setting] = value[opt_setting] where value is the opt_settings dictionary. However, the current code uses the outer value which is the entire opt_settings dictionary, making this line correct but confusing due to variable naming.

Suggested change
prob.driver.options[option] = value
for option, opt_settings in analysis_options["driver"]["options"].items():
if option == "opt_settings":
for opt_name in opt_settings:
prob.driver.opt_settings[opt_name] = opt_settings[opt_name]
else:
prob.driver.options[option] = opt_settings

Copilot uses AI. Check for mistakes.
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