-
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
Replace normalization and auto_normalize keyes with scaling and auto_scale in ObjectiveFunctionConfig #9965
Conversation
CodSpeed Performance ReportMerging #9965 will not alter performanceComparing Summary
|
b544d6a
to
9871c5d
Compare
9871c5d
to
f8070e4
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.
The idea is to change from a multiplication to a division factor. This is because we tend to have large objectives and then it is easier to specify the expected maximum value than its inverse. Division is also consistent with the scaling we use with constraints. So, in doc strings multiplication should be replaced with division, and in everest2ropt
the 1.0 / objective.scaling
should be replaced by objective.scaling
Otherwise looks good.
f8070e4
to
d44f900
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.
I think there also some test configuration yaml files that need changing?
I am guessing currently ropt is not designed to handled ropt_config["objectives"]["scales"] = [ 1.0 / objective.scaling] because if I add this change then the output looks like this for
where previously it was
which seems a bit more consistent because So I am a bit confused how both of these can be correct. |
d44f900
to
e5822b1
Compare
Seems there was a place in the everest_data_api where the single objective values where calculated and I had also to update the formulate there to divide by the scaling value and not multiply with the normalization. |
…scale in ObjectiveFunctionConfig
e5822b1
to
55a7074
Compare
It should be |
That seems correct. |
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 #9770
Approach
Short description of the approach
(Screenshot of new behavior in GUI if applicable)
git rebase -i main --exec 'just rapid-tests'
)When applicable