-
Notifications
You must be signed in to change notification settings - Fork 10
Improved OM driver interface #122
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
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 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_settingswithin 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": |
Copilot
AI
Aug 22, 2025
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.
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.
| if option == "opt_settings": | |
| if option == OPT_SETTINGS_KEY: |
| for opt_setting in value: | ||
| prob.driver.opt_settings[opt_setting] = value[opt_setting] | ||
| else: | ||
| prob.driver.options[option] = value |
Copilot
AI
Aug 22, 2025
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.
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.
| 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 |
Discovered changes to COBYLA implementation across
scipy1.15 and 1.16 that necessitateopt_settingsmanagement, this allows that from Ard.