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

Adding a new parameter to TPESampler #2385

Closed
jeromepatel opened this issue Feb 24, 2021 · 6 comments
Closed

Adding a new parameter to TPESampler #2385

jeromepatel opened this issue Feb 24, 2021 · 6 comments
Assignees
Labels
feature Change that does not break compatibility, but affects the public interfaces. stale Exempt from stale bot labeling.

Comments

@jeromepatel
Copy link
Contributor

Motivation

Currently, there is no argument to pass independent_sampler in TPESampler but in the future, it may be useful to users and may provide flexibility if we can add something as an argument to it like other samplers.

Description

In current implementation, There is a variable random_sampler there in TPESampler, whose after_trial method is called when a fallback occurs. Other samplers use independent_sampler by default initialized with RandomSampler, so in both the cases the performance is same if no argument is passed. However, if we include independent_sampler in TPESampler there will be flexibility when a new sampler can be used in place of RandomSampler (such as Sobol, issue #1797 ).

Additional context (optional)

This is discussion of independent_sampler in continuation of PR #2376. Let me know your views on this!

@jeromepatel jeromepatel added the feature Change that does not break compatibility, but affects the public interfaces. label Feb 24, 2021
@jeromepatel
Copy link
Contributor Author

I want to point out that currently, NSAGAIISampler and MOTPESampler don't have independent_sampler either, so we can consider them also (just for a reference of upgrade)

@HideakiImamura HideakiImamura self-assigned this Feb 26, 2021
@HideakiImamura
Copy link
Member

TPE supports independent sampling by default. So, it is inappropriate to add independent_sampler to TPESampler.

By the way, if the number of previous trials is smaller than n_startup_trials, the RandomSampler is used in TPESampler. I think it would be natural to allow this RandomSampler to be changed to any other sampler. For example, if SobolSampler or LatinHypercubeSampler are supported in the future, there is a good chance that we will want to replace it with something like that.

@HideakiImamura
Copy link
Member

I'd like to discuss the name of the argument to be added. independent_sampler is inappropriate as I said. How about init_sampler? Do you have any opinions?

@jeromepatel
Copy link
Contributor Author

jeromepatel commented Feb 26, 2021

@HideakiImamura Thank you for your suggestion. I agree with you on the point that TPESampler doesn't look appropriate with an argument named independent_sampler as it already supports it by default. As the proposal for a new sampler could improve flexibility (for eg. SobolSampler and others), the name init_sampler sounds perfect to me. I would like to know others' views on this.

@github-actions
Copy link
Contributor

This issue has not seen any recent activity.

@github-actions github-actions bot added the stale Exempt from stale bot labeling. label Mar 14, 2021
@github-actions
Copy link
Contributor

This issue was closed automatically because it had not seen any recent activity. If you want to discuss it, you can reopen it freely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Change that does not break compatibility, but affects the public interfaces. stale Exempt from stale bot labeling.
Projects
None yet
Development

No branches or pull requests

2 participants