Add `BisectSampler` #271

nabenabe0928 · 2025-05-23T04:32:34Z

Contributor Agreements

Please read the contributor agreements and if you agree, please click the checkbox below.

I agree to the contributor agreements.

Tip

Please follow the Quick TODO list to smoothly merge your PR.

Motivation

Description of the changes

TODO List towards PR Merge

Please remove this section if this PR is not an addition of a new package.
Otherwise, please check the following TODO list:

Copy ./template/ to create your package
Replace <COPYRIGHT HOLDER> in LICENSE of your package with your name
Fill out README.md in your package
Add import statements of your function or class names to be used in __init__.py
(Optional) Add from __future__ import annotations at the head of any Python files that include typing to support older Python versions
Apply the formatter based on the tips in README.md
Check whether your module works as intended based on the tips in README.md

HideakiImamura · 2025-05-28T03:13:13Z

@kAIto47802 Could you review this PR?

kAIto47802 · 2025-10-24T09:12:16Z

package/samplers/bisect/README.md

@@ -0,0 +1,56 @@
+---
+author: Shuhei Watanabe
+title: A Sampler Using Parameter-Wise Bisection, aka Binary, Search


Suggested change

title: A Sampler Using Parameter-Wise Bisection, aka Binary, Search

title: A Sampler Using Parameter-Wise Bisection, aka Binary Search

kAIto47802 · 2025-10-24T09:30:52Z

package/samplers/bisect/_sampler.py

+
+    def infer_relative_search_space(
+        self, study: optuna.Study, trial: optuna.trial.FrozenTrial
+    ) -> dict[str, optuna.distributions.BaseDistribution]:


The addition of optuna.distributions is inconsistent. We can remove it because BaseDistributions is already imported.

Suggested change

) -> dict[str, optuna.distributions.BaseDistribution]:

) -> dict[str, BaseDistribution]:

kAIto47802 · 2025-10-24T09:36:10Z

package/samplers/bisect/_sampler.py

+        param_name: str,
+        param_distribution: BaseDistribution,
+    ) -> Any:
+        if isinstance(param_distribution, optuna.distributions.CategoricalDistribution):


Same as above. Note that we have to add from optuna.distributions import CategoricalDistribution at the begging of this file.

Suggested change

if isinstance(param_distribution, optuna.distributions.CategoricalDistribution):

if isinstance(param_distribution, CategoricalDistribution):

kAIto47802 · 2025-10-24T09:48:16Z

package/samplers/bisect/_sampler.py

+        low = param_distribution.low
+        # The last element is padded to code the binary search routine cleaner.
+        high = param_distribution.high + step
+        assert step is not None


How about moving this assertion here, since step is already used in the addition operation, which doesn’t allow None?

Suggested change

low = param_distribution.low

# The last element is padded to code the binary search routine cleaner.

high = param_distribution.high + step

assert step is not None

assert step is not None

low = param_distribution.low

# The last element is padded to code the binary search routine cleaner.

high = param_distribution.high + step

kAIto47802 · 2025-10-24T10:31:35Z

package/samplers/bisect/_sampler.py

+        assert mid_index != len(possible_param_values) - 1, "The last element is for convenience."
+        return possible_param_values[mid_index].item()
+
+    def _get_possible_param_values(


I don’t think we need to list all possible parameter values for discrete distributions.
Doing so would make each sampling O(n_steps), which defeats the benefit of using binary search. Note that n_steps can be large, e.g., in trial.suggest_int("x", 0, 1 << 30, step=2).

We can avoid this by using the index of the discrete search space, as I’ll suggest in the alternative code below:

kAIto47802 · 2025-10-24T10:46:04Z

package/samplers/bisect/_sampler.py

+        possible_param_values = self._get_possible_param_values(dist)
+        indices = np.arange(len(possible_param_values))
+        left_index = indices[np.isclose(possible_param_values, left)][0]
+        right_index = indices[np.isclose(possible_param_values, right)][0]
+        return right_index - left_index <= 1


Suggested change

possible_param_values = self._get_possible_param_values(dist)

indices = np.arange(len(possible_param_values))

left_index = indices[np.isclose(possible_param_values, left)][0]

right_index = indices[np.isclose(possible_param_values, right)][0]

return right_index - left_index <= 1

left_index = int(np.round((left - dist.low) / dist.step))

right_index = int(np.round((right - dist.low) / dist.step))

return right_index - left_index <= 1

kAIto47802 · 2025-10-24T10:46:07Z

package/samplers/bisect/_sampler.py

+    def _get_possible_param_values(
+        self, param_distribution: FloatDistribution | IntDistribution
+    ) -> np.ndarray:
+        step = param_distribution.step
+        low = param_distribution.low
+        # The last element is padded to code the binary search routine cleaner.
+        high = param_distribution.high + step
+        assert step is not None
+        n_steps = int(np.round((high - low) / step)) + 1
+        return np.linspace(low, high, n_steps)
+


Suggested change

def _get_possible_param_values(

self, param_distribution: FloatDistribution | IntDistribution

) -> np.ndarray:

step = param_distribution.step

low = param_distribution.low

# The last element is padded to code the binary search routine cleaner.

high = param_distribution.high + step

assert step is not None

n_steps = int(np.round((high - low) / step)) + 1

return np.linspace(low, high, n_steps)

kAIto47802 · 2025-10-24T10:46:12Z

package/samplers/bisect/_sampler.py

+        possible_param_values = self._get_possible_param_values(param_distribution)
+        indices = np.arange(len(possible_param_values))
+        left_index = indices[np.isclose(possible_param_values, left)][0]
+        right_index = indices[np.isclose(possible_param_values, right)][0]
+        mid_index = (right_index + left_index) // 2
+        assert mid_index != len(possible_param_values) - 1, "The last element is for convenience."
+        return possible_param_values[mid_index].item()


Suggested change

possible_param_values = self._get_possible_param_values(param_distribution)

indices = np.arange(len(possible_param_values))

left_index = indices[np.isclose(possible_param_values, left)][0]

right_index = indices[np.isclose(possible_param_values, right)][0]

mid_index = (right_index + left_index) // 2

assert mid_index != len(possible_param_values) - 1, "The last element is for convenience."

return possible_param_values[mid_index].item()

left_index = int(np.round((left - param_distribution.low) / step))

right_index = int(np.round((right - param_distribution.low) / step))

mid_index = (left_index + right_index) // 2

return param_distribution.low + mid_index * step

kAIto47802 · 2025-10-24T11:04:17Z

Also, I still do not understand the motivation for adding this sampler.
Currently, this sampler searches the best variable value satisfying xxx_is_too_high, which is obvious and does not require any search algorithm, since it simply sets bounds for each variable directly.

The name BisectSampler gives me the impression that it performs a binary search on the user-provided objective function to find the best variable satisfying the given bounds, assuming that the objective function is monotonic. However, the current one doesn’t actually work that way.

kAIto47802 · 2025-10-24T11:12:24Z

package/samplers/bisect/example.py

+BisectSampler = optunahub.load_module("samplers/bisect").BisectSampler
+
+
+def objective(trial: optuna.Trial, score_func: Callable[[optuna.Trial], float]) -> float:


Why does it include the score_func argument, which will never be used and requires partial application before passed to study.optimize?

kAIto47802 · 2025-10-24T11:14:54Z

package/samplers/bisect/_sampler.py

+PREFIX_LEFT = "bisect:left_"
+PREFIX_RIGHT = "bisect:right_"


Suggested change

PREFIX_LEFT = "bisect:left_"

PREFIX_RIGHT = "bisect:right_"

_PREFIX_LEFT = "bisect:left_"

_PREFIX_RIGHT = "bisect:right_"

kAIto47802 · 2025-10-24T11:21:07Z

package/samplers/bisect/_sampler.py

+        n_steps = int(np.round((high - low) / step)) + 1
+        return np.linspace(low, high, n_steps)


Also, the calculation of the possible parameter values is incorrect.

nabenabe0928 added 3 commits May 23, 2025 06:31

Add bisect

e54f6db

Update README

cd28a17

Update

d94f657

c-bata assigned kAIto47802 Jul 7, 2025

kAIto47802 reviewed Oct 24, 2025

View reviewed changes

kAIto47802 suggested changes Oct 24, 2025

View reviewed changes

kAIto47802 reviewed Oct 24, 2025

View reviewed changes

kAIto47802 suggested changes Oct 24, 2025

View reviewed changes

	title: A Sampler Using Parameter-Wise Bisection, aka Binary, Search
	title: A Sampler Using Parameter-Wise Bisection, aka Binary Search

	) -> dict[str, optuna.distributions.BaseDistribution]:
	) -> dict[str, BaseDistribution]:

	if isinstance(param_distribution, optuna.distributions.CategoricalDistribution):
	if isinstance(param_distribution, CategoricalDistribution):

		BisectSampler = optunahub.load_module("samplers/bisect").BisectSampler


		def objective(trial: optuna.Trial, score_func: Callable[[optuna.Trial], float]) -> float:

		n_steps = int(np.round((high - low) / step)) + 1
		return np.linspace(low, high, n_steps)

Add BisectSampler #271

Are you sure you want to change the base?

Add BisectSampler #271

Uh oh!

Conversation

nabenabe0928 commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contributor Agreements

Motivation

Description of the changes

TODO List towards PR Merge

Uh oh!

HideakiImamura commented May 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kAIto47802 commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Add `BisectSampler` #271

Add `BisectSampler` #271

nabenabe0928 commented May 23, 2025 •

edited

Loading

kAIto47802 commented Oct 24, 2025 •

edited

Loading