-
Notifications
You must be signed in to change notification settings - Fork 39
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
Bugfix cec2022 benchmarks #16
Conversation
opfunu/utils/operator.py
Outdated
return np.abs(t2 - ndim) ** 0.25 + (0.5 * t2 + t1) / ndim + 0.5 | ||
|
||
|
||
def happy_cat_shifted_func(x): |
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.
Could you please add a 'shifted' parameter to this type of function? Kindly avoid fixed the shifted value within the function. This will make the function more reusable.
opfunu/utils/operator.py
Outdated
@@ -264,20 +310,23 @@ def hgbat_func(x): | |||
return np.abs(t2 ** 2 - t1 ** 2) ** 0.5 + (0.5 * t2 + t1) / ndim + 0.5 | |||
|
|||
|
|||
def hgbat_shifted_func(x): |
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.
This function too, you can add parameter for it.
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.
Please confirm you want parameterized after reading.
I specifically didn't go with a parameterized because I was thinking it would require either an if condition to check the parameter and shift or not, or always perform the shift just with a 0 for some cases if no shift is desired. While small this could introduce some extra processing. So having a shifted override function which calls the base unshifted seemed like the best performance vs clean approach.
That said if you feel going with a parameterized approach matches the codebase better I can go that direction, and if so prefer passing the shift value as 1, -1 etc. with a default of 0 instead of an if
check.
Thoughts?
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.
Yes, please set up the parameter name that match with function name. Also set up the default value for it. You don't need to check if else condition here. Your function will be reused more in future.
Other sections looking good. I will accept this PR after you do it. Thanks
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.
@thieu1995
Started to make the change and it doesn't seem right for the codebase.
Changing the signature to something like def happy_cat_func(x, shift=0.0)
works fine for when the benchmark is either calling the shifted or unshifted directly. ie. happy_cat_func(x, shift=-1.0)
or happy_cat_func(x)
. However with the composite benchmarks the code does something like self.g2 = operator.happy_cat_func
then in the evaluate we will get self.g2(mz[self.n1:self.n2], shift=-1.0)
which will be different from the other self.g1,g3 calls.
I considered a functional approach, which would be something like def happy_cat_func(shift=0.0): def inner(x): <existing logic>
which would then transform the self.g2 = operator.happy_cat_func(-1.0)
and the evaluate code would be the same. But then the functions that currently call the function directly would look like operator.happy_cat_func()(x)
which would be different.
The last alternative is leaving things the way I have with a separate function definition for the shifted case so the signatures in the benchmark stay the same.
So advise option 1, 2, or 3 given this information.
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 forgot to tell you when I defined self.g1 =..., self.g2=... in the init function. It is just for short code.
Now because you have parameter in the function, then you don't have to define g1, g2,... function.
Just write full function in evaluate().
For example, return operator.happy_cat_func((mz[self.n1:self.n2], shift=1.0) + operator.katsuura_func(mz[self.n1:self.n2], shift=-1.0,...) like that.
def levy_func(x): | ||
x = np.array(x).ravel() | ||
w = 1 + (x - 1) / 4 | ||
def levy_func(x, shift=0.0): |
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.
Found a reference for levy function, looks like the original is correct but that the expectation for the cec function was a 0 optimal shifted version. So went with a shift parameter like the hgbat and happy_cat with a default.
Thank you, If you have time, please take a look at other CEC set such as CEC2017 and CEC2014. |
Closes #
π Description
This change corrects the cec2022 benchmark as compared with published cec2022 source.
β Checks
βΉ Additional Information