Update `consider_prior` Behavior and Remove Support for `False` by sawa3030 · Pull Request #6007 · optuna/optuna

sawa3030 · 2025-03-12T04:34:01Z

Motivation

Update the behavior of consider_prior to always be True.
Ref: #6005.
note: Make consider_prior positional args in the following PR.

Description of Changes

Update the warning message to explicitly state that consider_prior is now always True.
Update the code to always consider prior.
Set consider_prior = True when constructing _ParzenEstimatorParameters.
Delete tests that previously verified behavior when consider_prior = False.

nabenabe0928

Thank you for the PR, I left some preliminary comments.

nabenabe0928 · 2025-03-12T04:48:05Z

optuna/samplers/_tpe/sampler.py

+            ) + ("From v4.3.0 onward, `consider_prior` is always `True`.")
            warnings.warn(msg, FutureWarning)


[nit] Automatic falling back is more exact in the meaning.

Suggested change

) + ("From v4.3.0 onward, `consider_prior` is always `True`.")

warnings.warn(msg, FutureWarning)

)

warnings.warn(

f"{msg} From v4.3.0 onward, `consider_prior` automatically falls back to `True`.",

FutureWarning,

)

nabenabe0928 · 2025-03-12T05:08:27Z

optuna/samplers/_tpe/parzen_estimator.py

+        if parameters.prior_weight is None:
+            raise ValueError("Prior weight must be specified because consider_prior==True.")
+        elif parameters.prior_weight <= 0:
+            raise ValueError("Prior weight must be positive.")


[nit] As consider_prior cannot be used anymore, we can merge these errors.

Suggested change

if parameters.prior_weight is None:

raise ValueError("Prior weight must be specified because consider_prior==True.")

elif parameters.prior_weight <= 0:

raise ValueError("Prior weight must be positive.")

if parameters.prior_weight is None or parameters.prior_weight <= 0:

raise ValueError(f"A positive value must be specified for prior_weight, but got {parameters.prior_weight}.")

nabenabe0928 · 2025-03-12T05:11:10Z

optuna/samplers/_tpe/parzen_estimator.py

        if len(transformed_observations) == 0:
            weights = np.array([1.0])
-        elif parameters.consider_prior:
+        else:
            assert parameters.prior_weight is not None
            weights = np.append(weights, [parameters.prior_weight])


As prior_weight is already guaranteed to be not None from the earlier line, we can simply code like this:

weights = np.append(weights, [parameters.prior_weight])

nabenabe0928 · 2025-03-12T05:12:30Z

optuna/samplers/_tpe/parzen_estimator.py

+        consider_prior = True
+        n_kernels = len(observations) + consider_prior


[nit] An inline comment is sufficient.

Suggested change

consider_prior = True

n_kernels = len(observations) + consider_prior

n_kernels = len(observations) + 1 # NOTE(sawa): +1 for prior.

nabenabe0928 · 2025-03-12T05:14:50Z

optuna/samplers/_tpe/parzen_estimator.py

            maxsigma = 1.0 * (high - low + step_or_0)
            if parameters.consider_magic_clip:
                # TODO(contramundum53): Remove dependency of minsigma on consider_prior.
+                consider_prior = True


What about it?

Suggested change

consider_prior = True

n_kernels = len(observations) + 1 # NOTE(sawa): +1 for prior.

nabenabe0928 · 2025-03-12T05:15:53Z

optuna/samplers/_tpe/parzen_estimator.py

+        prior_mu = 0.5 * (low + high)
+        prior_sigma = 1.0 * (high - low + step_or_0)
+        mus = np.append(mus, [prior_mu])
+        sigmas = np.append(sigmas, [prior_sigma])


[nit] Reduced definitions of unused variables.

Suggested change

prior_mu = 0.5 * (low + high)

prior_sigma = 1.0 * (high - low + step_or_0)

mus = np.append(mus, [prior_mu])

sigmas = np.append(sigmas, [prior_sigma])

# Add the stats for non-informative prior.

mus = np.append(mus, [0.5 * (low + high)])

sigmas = np.append(sigmas, [1.0 * (high - low + step_or_0)])

optuna/samplers/_tpe/parzen_estimator.py

nabenabe0928 · 2025-03-12T07:27:49Z

tests/samplers_tests/tpe_tests/test_parzen_estimator.py

        ],
        [
            np.asarray([-0.4, 0.4, 0.41, 0.42]),
-            {"prior": False, "magic_clip": True, "endpoints": True},


I believe we can remove unit tests for prior=False as the tests for prior=True already cover the functionality.
But please check carefully whether the removed tests are really covered by the existing tests.

Thank you for your review. Since this is the only test in test_calculate that uses magic_clip = True, I believe it should be retained.

sawa3030 · 2025-03-14T01:51:56Z

@nabenabe0928 Thank you for your advice. Everything should be updated now. PTAL!

c-bata · 2025-03-17T01:58:14Z

@gen740 @nabenabe0928 Could you review this PR?

nabenabe0928 · 2025-03-19T08:00:46Z

optuna/importance/_ped_anova/scott_parzen_estimator.py

    # counts.astype(float) is necessary for weight calculation in ParzenEstimator.
    return _ScottParzenEstimator(
-        param_name, rounded_dist, counts.astype(np.float64), consider_prior, prior_weight
+        param_name, rounded_dist, counts.astype(np.float64), True, prior_weight


[nit] It would be better to pass arguments with keys as consider_prior is hard-coded.

Suggested change

param_name, rounded_dist, counts.astype(np.float64), True, prior_weight

param_name=param_name,

dist=rounded_dist,

counts=counts.astype(np.float64),

consider_prior=True,

prior_weight=prior_weight,

nabenabe0928 · 2025-03-19T09:21:13Z

optuna/samplers/_tpe/sampler.py


        self._parzen_estimator_parameters = _ParzenEstimatorParameters(
-            consider_prior,
+            True,


Could you use the keyword args here for more clarity?

Suggested change

True,

consider_prior=True,

nabenabe0928 · 2025-03-19T09:30:44Z

tests/samplers_tests/tpe_tests/test_parzen_estimator.py

    )
-    assert len(mpe._mixture_distribution.weights) == max(len(mus) + int(prior), 1)
+    assert len(mpe._mixture_distribution.weights) == max(
+        len(mus) + 1, 1


We don't need the max operator.

Suggested change

len(mus) + 1, 1

len(mus) + 1

nabenabe0928

I left some comments except for the unit tests of PED-ANOVA.

nabenabe0928 · 2025-03-19T09:31:19Z

tests/samplers_tests/tpe_tests/test_parzen_estimator.py

    )
-    assert len(mpe._mixture_distribution.weights) == max(len(mus) + int(prior), 1)
+    assert len(mpe._mixture_distribution.weights) == max(
+        len(mus) + 1, 1


Same here. We don't need the max operator.

Suggested change

len(mus) + 1, 1

len(mus) + 1

c-bata · 2025-03-21T09:27:32Z

Let me reassign the reviewer since gen740 is busy for a while.
@not522 Could you review this PR?

sawa3030 · 2025-03-23T23:53:53Z

@nabenabe0928 Thank you for your review. I updated the code based on your suggestion and I also made some changes to the unit tests of PED-ANOVA. PTAL!

sawa3030 · 2025-03-27T03:33:35Z

@nabenabe0928 @not522 I would really appreciate it if you could take a moment to review the code. Thank you!

not522

How about remove consider_prior from _ParzenEstimatorParameters? Since it is an internal class, we can break the compatibility.

…mator

sawa3030 · 2025-03-28T06:02:05Z

@not522 Thank you for your advice. I decided to remove consider_prior from _ParzenEstimatorParameters, _ParzenEstimator, and _ScottParzenEstimator. PTAL!

not522 · 2025-03-28T07:29:56Z

optuna/samplers/_tpe/parzen_estimator.py


 class _ParzenEstimatorParameters(NamedTuple):
-    consider_prior: bool
    prior_weight: float | None


How about remove None from the prior_weight type?

Suggested change

prior_weight: float | None

prior_weight: float

nabenabe0928 · 2025-03-28T10:49:34Z

optuna/samplers/_tpe/sampler.py

            )

        self._parzen_estimator_parameters = _ParzenEstimatorParameters(
-            consider_prior=True,


Could you simply remove only this line and keep the keyword argument as is?

nabenabe0928 · 2025-03-28T10:51:47Z

optuna/importance/_ped_anova/evaluator.py

-        # Prior is used for regularization.
-        self._consider_prior = True
-        # Control the regularization effect.
+        # Control the regularization effect of prior.


Suggested change

# Control the regularization effect of prior.

# Control the regularization effect by prior.

nabenabe0928 · 2025-03-28T11:06:46Z

tests/importance_tests/pedanova_tests/test_scott_parzen_estimator.py

I still need to check the changes in this file.

sawa3030 · 2025-04-02T00:30:06Z

I’ve made some modifications based on your suggestion. PTAL! @not522 @nabenabe0928

not522

Thank you for your update. It's almost LGTM.
I left nit comments. Please check them.

not522 · 2025-04-02T23:38:53Z

optuna/samplers/_tpe/parzen_estimator.py

-                raise ValueError("Prior weight must be specified when consider_prior==True.")
-            elif parameters.prior_weight <= 0:
-                raise ValueError("Prior weight must be positive.")
+        if parameters.prior_weight is None or parameters.prior_weight <= 0:


Suggested change

if parameters.prior_weight is None or parameters.prior_weight <= 0:

if parameters.prior_weight <= 0:

not522 · 2025-04-02T23:58:31Z

optuna/samplers/_tpe/parzen_estimator.py


-        n_kernels = len(observations) + parameters.consider_prior
+        n_kernels = len(observations) + 1  # NOTE(sawa3030): +1 for prior.
        assert parameters.prior_weight is not None


Suggested change

assert parameters.prior_weight is not None

sawa3030 · 2025-04-03T03:02:04Z

@not522 Thank you for your support. I've made the updates accordingly.

not522

LGTM!

nabenabe0928 · 2025-04-07T04:13:20Z

tests/importance_tests/pedanova_tests/test_scott_parzen_estimator.py

        counts=counts,
-        consider_prior=False,
-        prior_weight=0.0,
+        prior_weight=1.0,


I believe this part should still test prior_weight=0.0 to check whether PED-ANOVA works with prior_weight==0.0.

nabenabe0928 · 2025-04-07T04:15:22Z

tests/importance_tests/pedanova_tests/test_scott_parzen_estimator.py

        counts=_counts,
-        consider_prior=False,
-        prior_weight=0.0,
+        prior_weight=1.0,


Suggested change

prior_weight=1.0,

prior_weight=0.0,

nabenabe0928

Sorry for the late response 🙇
I left some comments on the test file for PED-ANOVA.
Basically, PED-ANOVA tests prior_weight=0.0 just because the original paper uses prior_weight=0.0 actually.

nabenabe0928 · 2025-04-07T04:15:51Z

tests/importance_tests/pedanova_tests/test_scott_parzen_estimator.py

        counts=_counts,
-        consider_prior=False,
-        prior_weight=0.0,
+        prior_weight=1.0,


Suggested change

prior_weight=1.0,

prior_weight=0.0,

sawa3030 · 2025-04-09T00:10:08Z

@nabenabe0928 I modified _ParzenEstimator to support prior_weight = 0.0. However, I'm a bit concerned that this change implicitly allow consider_prior = False. Thank you very much for your continued support.

nabenabe0928 · 2025-04-10T06:40:23Z

I'm a bit concerned that this change implicitly allows consider_prior = False.

In fact, PED-ANOVA was originally expected to be used without prior and prior_weight=0.0 is an edge case, so it should be tested anyways although only advanced users use prior_weight=0.0.

sawa3030 · 2025-04-10T23:52:57Z

@nabenabe0928 Thank you for your advice. I think I made the necessary updates. PTAL!

nabenabe0928

Thank you very much for the hard work on this PR, LGTM:)

sawa3030 added 4 commits March 10, 2025 23:55

set consider_prior true

d995bfd

minor change

b02cfae

Merge branch 'follow-pep8' into set-consider-prior-true

538bbd1

apply consider_prior = True to tests

a55bd88

nabenabe0928 reviewed Mar 12, 2025

View reviewed changes

sawa3030 added 3 commits March 12, 2025 15:27

refactoring

e8698d1

refactoring

db2c617

minor change

ebd0302

nabenabe0928 reviewed Mar 12, 2025

View reviewed changes

sawa3030 marked this pull request as ready for review March 13, 2025 01:09

c-bata assigned gen740 and nabenabe0928 Mar 17, 2025

nabenabe0928 reviewed Mar 19, 2025

View reviewed changes

address comments

7bb8e34

c-bata assigned not522 and unassigned gen740 Mar 21, 2025

minor change

be6027f

not522 reviewed Mar 27, 2025

View reviewed changes

remove consider_prior from _ParzenEstimatorParameters nad _ParzenEsti…

9ac5e7e

…mator

c-bata added the compatibility Change that breaks compatibility. label Mar 28, 2025

minor change

d01d9e2

not522 reviewed Mar 28, 2025

View reviewed changes

nabenabe0928 reviewed Mar 28, 2025

View reviewed changes

minor change

94b7c53

not522 reviewed Apr 3, 2025

View reviewed changes

minor change

d61da24

not522 approved these changes Apr 3, 2025

View reviewed changes

not522 removed their assignment Apr 3, 2025

nabenabe0928 reviewed Apr 7, 2025

View reviewed changes

sawa3030 force-pushed the set-consider-prior-true branch from 6be7b57 to a8e0ab2 Compare April 8, 2025 12:34

allow prior_weight == 0

6329097

sawa3030 force-pushed the set-consider-prior-true branch from a8e0ab2 to 6329097 Compare April 8, 2025 23:18

nabenabe0928 mentioned this pull request Apr 10, 2025

Add the init constructor to the TPE-related samplers optuna/optunahub-registry#263

Merged

sawa3030 added 3 commits April 10, 2025 23:45

minor change

c938fc7

minor change

06b4d7f

minor change

d8cfbb9

nabenabe0928 approved these changes Apr 11, 2025

View reviewed changes

nabenabe0928 merged commit 5770d7a into optuna:master Apr 11, 2025
14 checks passed

sawa3030 mentioned this pull request Apr 14, 2025

Make all arguments of TPESampler keyword-only #6041

Merged

nabenabe0928 added this to the v4.4.0 milestone Apr 16, 2025

sawa3030 deleted the set-consider-prior-true branch November 14, 2025 07:51

		) + ("From v4.3.0 onward, `consider_prior` is always `True`.")
		warnings.warn(msg, FutureWarning)

		consider_prior = True
		n_kernels = len(observations) + consider_prior

	consider_prior = True
	n_kernels = len(observations) + consider_prior
	n_kernels = len(observations) + 1 # NOTE(sawa): +1 for prior.

-        param_name, rounded_dist, counts.astype(np.float64), True, prior_weight
+        param_name=param_name,
+        dist=rounded_dist,
+        counts=counts.astype(np.float64),
+        consider_prior=True,
+        prior_weight=prior_weight,

	# Control the regularization effect of prior.
	# Control the regularization effect by prior.

	if parameters.prior_weight is None or parameters.prior_weight <= 0:
	if parameters.prior_weight <= 0:

Uh oh!

Conversation

sawa3030 commented Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Description of Changes

Uh oh!

nabenabe0928 left a comment

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!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sawa3030 commented Mar 14, 2025

Uh oh!

c-bata commented Mar 17, 2025

Uh oh!

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

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!

nabenabe0928 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

c-bata commented Mar 21, 2025

Uh oh!

sawa3030 commented Mar 23, 2025

Uh oh!

sawa3030 commented Mar 27, 2025

Uh oh!

not522 left a comment

Choose a reason for hiding this comment

Uh oh!

sawa3030 commented Mar 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!

sawa3030 commented Apr 2, 2025

Uh oh!

not522 left a comment

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!

sawa3030 commented Apr 3, 2025

Uh oh!

not522 left a comment

Choose a reason for hiding this comment

sawa3030 commented Mar 12, 2025 •

edited

Loading

nabenabe0928 Mar 12, 2025 •

edited

Loading

nabenabe0928 Mar 12, 2025 •

edited

Loading

nabenabe0928 Mar 19, 2025 •

edited

Loading