Skip to content

Update vsbx#6033

Merged
y0z merged 5 commits intooptuna:masterfrom
hrntsm:update-vsbx
Apr 21, 2025
Merged

Update vsbx#6033
y0z merged 5 commits intooptuna:masterfrom
hrntsm:update-vsbx

Conversation

@hrntsm
Copy link
Copy Markdown
Contributor

@hrntsm hrntsm commented Apr 7, 2025

Motivation

Improve vSBX based on #6000, #6008

Description of the changes

I have changed the implementation to make it possible to create a distribution of child individuals as in the reference paper.
See #6000 for details.

In order to align the paper with the results, the implementation has been changed and the crossover results are no longer compatible with the previous results. Therefore, the test is being rewritten.
I understand that this is not desirable for Optuna.
On the other hand, the fact that the implementation does not match the reference paper is considered a bug, so I create this PR.

Any comments on what form it should take would be appreciated.

BLXAlphaCrossover(),
SPXCrossover(),
SBXCrossover(),
VSBXCrossover(),
Copy link
Copy Markdown
Contributor Author

@hrntsm hrntsm Apr 7, 2025

Choose a reason for hiding this comment

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

Removed because even if the parent has the same variables, the child is created in its neighborhood and the purpose of this test, to be the same individual as the parent, is not achieved.

@y0z y0z self-assigned this Apr 7, 2025
@y0z
Copy link
Copy Markdown
Member

y0z commented Apr 7, 2025

@sawa3030 Could you review this PR? (cc @HideakiImamura )

@y0z y0z added the bug Issue/PR about behavior that is broken. Not for typos/examples/CI/test but for Optuna itself. label Apr 7, 2025
uniform_crossover_prob: float = 0.5,
use_child_gene_prob: float = 0.5,
) -> None:
self._eta = eta
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is the valid range for $\eta$ is $\geq 0$? I want to validate the value here since I find that setting $\eta = -1$ causes ZeroDivisionError. This may also happen in SBXCrossOver.

    beta_1 = np.power(1 / (2 * us), 1 / (eta + 1))
                                    ~~^~~~~~~~~~~
ZeroDivisionError: division by zero

Copy link
Copy Markdown
Contributor Author

@hrntsm hrntsm Apr 8, 2025

Choose a reason for hiding this comment

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

I could not find a description of the range of eta in the references in the document, but the following ones mention that it is a non-negative value.
Add a check for non-negative values, since there is also the issue of zero divisions.

Self-adaptive simulated binary crossover for real-parameter optimization

I will implement the same for the SBX as a another PR.

)

# vSBX applies crossover with establishment 0.5, and with probability 0.5,
beta_1 = np.power(1 / (2 * us), 1 / (eta + 1))
Copy link
Copy Markdown
Member

@y0z y0z Apr 8, 2025

Choose a reason for hiding this comment

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

Although it is a very small probability, there is a chance of becoming the denominator in 1 / (2 * us) can be zero since rng.rand() $\in [0, 1)$ (ref).

The below might be safer. (This pseudocode might require modification since us is vector whereas eps is scalar.)

        beta_1 = np.power(1 / max((2 * us), eps), 1 / (eta + 1))

Copy link
Copy Markdown
Contributor Author

@hrntsm hrntsm Apr 8, 2025

Choose a reason for hiding this comment

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

There is no strong reason for the eps, but I have chosen to use the same values used in the SBX.

xs_diff = np.clip(xs_max - xs_min, 1e-10, None)

Although out of the range of values actually generated(rng.rand()∈[0,1)), 1 is set as us value in the test. Since zero divisions occur even in beta_2, a similar correction has been made.

beta_2 = np.power(1 / (2 * (1 - us)), 1 / (eta + 1))

(
VSBXCrossover(),
0.0,
np.array([-1076.02679423, -2151.84728898]),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Question: Where do these values come from? I have no idea about what is tested here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Based on #6033 (comment), I modified the following so that zero divisions do not occur.

beta_1 = np.power(1 / np.maximum((2 * us), eps), 1 / (eta + 1))

The original test was [-inf, -inf] because of the zero division, but has been corrected as above.

However, beta_1 still be a very large value, so vsbx generate a child on the -inf side. The value tested here is considered to be a result that depends on the eps value.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I followed the original test with a random value of 0 and adjusted the test results, but as you pointed out, this is not a very meaningful test, so it may be more appropriate to specify a different meaningful random value.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it's important to keep the test with us = 0 since it covers an edge case. That said, I agree that the test is ambiguous. How about adding a comment to clarify the behavior, as you mentioned here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added a comment on the test content.

Copy link
Copy Markdown
Collaborator

@sawa3030 sawa3030 left a comment

Choose a reason for hiding this comment

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

Thank you for the update. LGTM!

Copy link
Copy Markdown
Member

@y0z y0z left a comment

Choose a reason for hiding this comment

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

LGTM

@y0z y0z added this to the v4.4.0 milestone Apr 21, 2025
@y0z y0z merged commit 631b576 into optuna:master Apr 21, 2025
14 checks passed
@hrntsm hrntsm deleted the update-vsbx branch April 21, 2025 06:32
@hrntsm hrntsm mentioned this pull request Apr 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issue/PR about behavior that is broken. Not for typos/examples/CI/test but for Optuna itself.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants