-
Notifications
You must be signed in to change notification settings - Fork 110
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
Consolidate optimizer options and backend_options #9887
Consolidate optimizer options and backend_options #9887
Conversation
CodSpeed Performance ReportMerging #9887 will degrade performances by 10.42%Comparing Summary
Benchmarks breakdown
|
9c42031
to
8b58f7e
Compare
if backend == "dakota": | ||
alg_const_tol = ever_opt.constraint_tolerance or None | ||
if has_output_constraints and alg_const_tol is not None: | ||
options += [f"constraint_tolerance = {alg_const_tol}"] |
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.
Seems to only be for strs beforehand, should it also be for dicts?
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.
We do have a problem here. That constraint_tolerance
line is dakota specific, so we should only add it if the backend is dakota. For now, do indeed just add it when the options are a list. Not for dicts.
ab45054
to
b29f041
Compare
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.
Looks good, but needs a deprecation message.
if backend == "dakota": | ||
alg_const_tol = ever_opt.constraint_tolerance or None | ||
if has_output_constraints and alg_const_tol is not None: | ||
options += [f"constraint_tolerance = {alg_const_tol}"] |
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.
We do have a problem here. That constraint_tolerance
line is dakota specific, so we should only add it if the backend is dakota. For now, do indeed just add it when the options are a list. Not for dicts.
b29f041
to
a10422e
Compare
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.
LGTM!
Issue
Resolves #9772
Approach
Pass string to dakota, otherwise pass a dict.