-
-
Notifications
You must be signed in to change notification settings - Fork 94
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
Change the definition of union_search_space
, intersect_search_space
#1026
Change the definition of union_search_space
, intersect_search_space
#1026
Conversation
union_search_space
union_search_space
, intersect_search_space
…x/trial-table-duplicates-values
for n1, d1 in trial.distributions.items(): | ||
for n2, d2 in self._union_search_space: | ||
if n1 == n2: | ||
if _is_same_float_distribution(d1, d2): |
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'm just wondering if there's a better approach to mitigate this deeply nested code. Do you think we could define functions like the following to make the code easier to read?
def get_union_search_space(space_1, space_2) -> ...:
...
def get_intersection_search_space(space_1, space_2) -> ...:
...
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 reduced the nesting by one level, but I couldn't think of a good way to simplify it further...
The functions you mentioned might not be the best since original code requires break
inside an if
statement.
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! Thank you for your contribution.
Contributor License Agreement
This repository (
optuna-dashboard
) and Goptuna share common code.This pull request may therefore be ported to Goptuna.
Make sure that you understand the consequences concerning licenses and check the box below if you accept the term before creating this pull request.
Reference Issues/PRs
Fixes #1012
What does this implement/fix? Explain your changes.
The following changes have been made:
search_space
remains the same.TrialTable
where parameters with the same name resulted in identical row IDs, causing duplicate key props and triggering a React warning.