Skip to content
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

Fix correction of out-of-range discrete params of CMAwM #136

Merged
merged 2 commits into from
Dec 26, 2022

Conversation

knshnb
Copy link
Contributor

@knshnb knshnb commented Dec 22, 2022

A discrete version of #134.

@knshnb
Copy link
Contributor Author

knshnb commented Dec 22, 2022

@nomuramasahir0 Let me confirm one point related to this PR.
When we have a discrete parameter of step 1 with bounds = [1.0, 3.0], should we sample the continuous version of the parameter from [1.0, 3.0] or [0.5, 3.5]?

@nomuramasahir0
Copy link
Collaborator

@knshnb
To be consistent with the continuous case in the current implementation, I think it may be better to use [1.0, 3.0] ver also in the discrete case.
But to be honestly, I'm not so sure whether this is reasonable, and so this opinion may change in the future.
In the near future, I would like to empirically investigate reasonable box constraint handling in the continuous and discrete case in the CMA-ES.

Copy link
Collaborator

@c-bata c-bata left a comment

Choose a reason for hiding this comment

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

To be consistent with the continuous case in the current implementation, I think it may be better to use [1.0, 3.0] ver also in the discrete case.

Thank you for your input! LGTM.

@knshnb
Copy link
Contributor Author

knshnb commented Dec 26, 2022

Thanks for the reply and review!
Then, let me proceed in this direction and delete the method _repair_continuous_params, which is no longer necessary.

@knshnb knshnb changed the title [WIP] Fix correction of out-of-range discrete params of CMAwM Fix correction of out-of-range discrete params of CMAwM Dec 26, 2022
@c-bata
Copy link
Collaborator

c-bata commented Dec 26, 2022

Let me merge this PR since we reached a consensus with @nomuramasahir0. Thank you for the input.

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.

3 participants