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 param_mask for multivariate TPE with constant_liar #4462

Merged
merged 1 commit into from
Mar 2, 2023

Conversation

not522
Copy link
Member

@not522 not522 commented Feb 24, 2023

Motivation

Fix #4459.

When multivariate=True and constant_liar=True, running trials may partially sample parameters in a relative search space. In this case, we need to check all parameters to make the mask.

Description of the changes

Check all parameters to make param_mask.

@not522 not522 added the bug Issue/PR about behavior that is broken. Not for typos/examples/CI/test but for Optuna itself. label Feb 24, 2023
@github-actions github-actions bot added the optuna.samplers Related to the `optuna.samplers` submodule. This is automatically labeled by github-actions. label Feb 24, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #4462 (d17c526) into master (d9a8850) will decrease coverage by 0.74%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master    #4462      +/-   ##
==========================================
- Coverage   90.40%   89.66%   -0.74%     
==========================================
  Files         172      178       +6     
  Lines       13682    13974     +292     
==========================================
+ Hits        12369    12530     +161     
- Misses       1313     1444     +131     
Impacted Files Coverage Δ
optuna/samplers/_tpe/sampler.py 96.71% <100.00%> (ø)
optuna/integration/botorch.py 95.63% <0.00%> (-0.80%) ⬇️
optuna/study/study.py 94.23% <0.00%> (-0.74%) ⬇️
optuna/samplers/_cmaes.py 98.25% <0.00%> (ø)
optuna/terminator/__init__.py 44.44% <0.00%> (ø)
optuna/terminator/improvement/evaluator.py 89.28% <0.00%> (ø)
optuna/terminator/improvement/gp/botorch.py 44.11% <0.00%> (ø)
optuna/terminator/improvement/_preprocessing.py 34.48% <0.00%> (ø)
optuna/terminator/_search_space/intersection.py 100.00% <0.00%> (ø)
optuna/terminator/improvement/gp/base.py 91.66% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@knshnb knshnb left a comment

Choose a reason for hiding this comment

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

I confirmed the errors in both codes (#4459 (comment), #4459 (comment)) are resolved! LGTM.

@not522
Copy link
Member Author

not522 commented Feb 28, 2023

I found #4079 accidentally introduced this bug. So, versions before 3.1.0 do not have it.

Copy link
Contributor

@cross32768 cross32768 left a comment

Choose a reason for hiding this comment

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

I also confirmed the original error is fixed and your test works to confirm this fix. LGTM!

@cross32768 cross32768 merged commit e4baab8 into optuna:master Mar 2, 2023
@not522 not522 deleted the fix-multivariate-constantliar branch March 5, 2023 23:27
contramundum53 pushed a commit that referenced this pull request Apr 3, 2023
Fix `param_mask` for multivariate TPE with `constant_liar`
HideakiImamura added a commit that referenced this pull request Apr 3, 2023
@toshihikoyanase toshihikoyanase added this to the v3.2.0 milestone Apr 21, 2023
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. optuna.samplers Related to the `optuna.samplers` submodule. This is automatically labeled by github-actions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Index out of bounds error in TpeSampler when multivariate=constant_liar=True and parallel jobs
5 participants