Skip to content

Fix log PDF of discrete trunc log-norm distribution for TPESampler#6258

Merged
y0z merged 5 commits intooptuna:masterfrom
not522:fix-tpe-logint
Nov 17, 2025
Merged

Fix log PDF of discrete trunc log-norm distribution for TPESampler#6258
y0z merged 5 commits intooptuna:masterfrom
not522:fix-tpe-logint

Conversation

@not522
Copy link
Copy Markdown
Member

@not522 not522 commented Aug 25, 2025

Motivation

TPESampler exhibits inconsistent handling of suggest_int(..., log=True). While the Parzen estimator uses the log-normal distribution's interval during sampling, it employs the log-normal distribution's value at the sampled point for the log PDF. This PR fixes the behavior to use the log-normal distribution's interval for the log PDF as well.

Description of the changes

  • The first commit (4ede700) introduced _BatchedTruncLogNormDistributions and _BatchedDiscreteTruncLogNormDistributions to clarify the behavior of each distribution. It is a refactoring and does not change the behavior.
  • The second commit (a5ab97c) fixed the log PDF of _BatchedDiscreteTruncLogNormDistributions.

Using the code below, I confirmed that the optimization performance remained nearly unchanged.

import numpy as np
import matplotlib.pyplot as plt
import optuna

def objective(trial):
    params = np.empty(5)
    for i in range(5):
        params[i] = trial.suggest_int(f"x{i}", 1, 100, log=True)
    return np.sum((np.log(params) - np.log(10)) ** 2)

N_STUDIES = 100
N_TRIALS = 100

y = [[] for _ in  range(N_STUDIES)]
for seed in range(N_STUDIES):
    sampler = optuna.samplers.TPESampler(seed=seed)
    study = optuna.create_study(sampler=sampler)
    study.optimize(objective, n_trials=N_TRIALS)
    y[seed].append(study.trials[0].value)
    for i in range(1, N_TRIALS):
        y[seed].append(min(y[seed][-1], study.trials[i].value))

y = np.asarray(y)
np.save("master.npy", y)
# np.save("pr.npy", y)
y = np.load("master.npy")
mean = np.mean(y, axis=0)
std = np.std(y, axis=0)
plt.fill_between(range(N_TRIALS), mean + std, mean - std, alpha=0.1)
plt.plot(range(N_TRIALS), mean, label="master")

y = np.load("pr.npy")
mean = np.mean(y, axis=0)
std = np.std(y, axis=0)
plt.fill_between(range(N_TRIALS), mean + std, mean - std, alpha=0.1)
plt.plot(range(N_TRIALS), mean, label="fixed")

plt.xlabel("trial number")
plt.ylabel("loss")
plt.legend()
plt.savefig("logint.png")
logint

@not522 not522 added the bug Issue/PR about behavior that is broken. Not for typos/examples/CI/test but for Optuna itself. label Aug 25, 2025
@nabenabe0928 nabenabe0928 self-assigned this Aug 25, 2025
@nabenabe0928 nabenabe0928 added this to the v4.6.0 milestone Aug 25, 2025
@not522
Copy link
Copy Markdown
Member Author

not522 commented Aug 26, 2025

After examining the behavior of TPESampler, I discovered that suggest_int(..., log=True) exhibits a bias toward sampling smaller values. To properly address this issue, I will temporarily convert this PR to draft status.

@not522 not522 marked this pull request as draft August 26, 2025 07:42
@not522
Copy link
Copy Markdown
Member Author

not522 commented Aug 27, 2025

This PR introduces the following behavioral changes, so I've confirmed that results may vary depending on the objective function.

  • The original implementation tends to sample larger values.
  • This PR makes sigmas greater.

While there may be room for discussion about how probability distributions should be determined, I think this PR can be treated as bug fixes.

====

def objective(trial):
    params = np.empty(5)
    for i in range(5):
        params[i] = trial.suggest_int(f"x{i}", 1, 100, log=True)
    return np.sum((np.log(params) - np.log(1)) ** 2)
logint1
def objective(trial):
    params = np.empty(20)
    for i in range(20):
        params[i] = trial.suggest_int(f"x{i}", 1, 100, log=True)
    return np.sum((np.log(params) - np.log(10)) ** 2)
logint20

@not522 not522 marked this pull request as ready for review August 27, 2025 04:16
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Sep 4, 2025

This pull request has not seen any recent activity.

@github-actions github-actions bot added the stale Exempt from stale bot labeling. label Sep 4, 2025
@nabenabe0928 nabenabe0928 removed the stale Exempt from stale bot labeling. label Sep 5, 2025
@github-actions
Copy link
Copy Markdown
Contributor

This pull request has not seen any recent activity.

@github-actions github-actions bot added the stale Exempt from stale bot labeling. label Sep 14, 2025
@nabenabe0928 nabenabe0928 removed the stale Exempt from stale bot labeling. label Sep 15, 2025
@github-actions
Copy link
Copy Markdown
Contributor

This pull request has not seen any recent activity.

@github-actions github-actions bot added the stale Exempt from stale bot labeling. label Sep 22, 2025
@nabenabe0928 nabenabe0928 removed the stale Exempt from stale bot labeling. label Sep 25, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Oct 2, 2025

This pull request has not seen any recent activity.

@github-actions github-actions bot added the stale Exempt from stale bot labeling. label Oct 2, 2025
@nabenabe0928 nabenabe0928 removed the stale Exempt from stale bot labeling. label Oct 3, 2025
@github-actions
Copy link
Copy Markdown
Contributor

This pull request has not seen any recent activity.

@github-actions github-actions bot added the stale Exempt from stale bot labeling. label Oct 12, 2025
@nabenabe0928 nabenabe0928 removed the stale Exempt from stale bot labeling. label Oct 14, 2025
@github-actions
Copy link
Copy Markdown
Contributor

This pull request has not seen any recent activity.

@github-actions github-actions bot added the stale Exempt from stale bot labeling. label Oct 21, 2025
@c-bata c-bata removed the stale Exempt from stale bot labeling. label Oct 22, 2025
Comment on lines +96 to +97
lows_cont = []
highs_cont = []
Copy link
Copy Markdown
Collaborator

@kAIto47802 kAIto47802 Oct 22, 2025

Choose a reason for hiding this comment

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

Suggested change
lows_cont = []
highs_cont = []
lows_num, highs_num = [], []

How about renaming it to "num", since naming "cont" for both continuous and discrete distributions is a bit misleading.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

How about numeric? (num might be confusing with number)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for your response.
That's right. numeric seems to be better.

Suggested change
lows_cont = []
highs_cont = []
lows_number, highs_number = [], []

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thank you. I've updated it.

Copy link
Copy Markdown
Collaborator

@kAIto47802 kAIto47802 left a comment

Choose a reason for hiding this comment

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

Thank you for the PR!
It's almost LGTM, leaving only a minor comment.

@y0z y0z removed this from the v4.6.0 milestone Oct 30, 2025
Copy link
Copy Markdown
Collaborator

@kAIto47802 kAIto47802 left a comment

Choose a reason for hiding this comment

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

Thank you for the update! LGTM

@kAIto47802 kAIto47802 removed their assignment Oct 31, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Nov 9, 2025

This pull request has not seen any recent activity.

@github-actions github-actions bot added the stale Exempt from stale bot labeling. label Nov 9, 2025
@c-bata c-bata removed the stale Exempt from stale bot labeling. label Nov 12, 2025
@not522 not522 assigned y0z and unassigned nabenabe0928 Nov 12, 2025
@not522
Copy link
Copy Markdown
Member Author

not522 commented Nov 12, 2025

@y0z Could you review this PR?

Copy link
Copy Markdown
Member

@y0z y0z left a comment

Choose a reason for hiding this comment

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

LGTM

@y0z y0z merged commit 0297d1b into optuna:master Nov 17, 2025
12 checks passed
@y0z y0z removed their assignment Nov 17, 2025
@not522 not522 deleted the fix-tpe-logint branch November 18, 2025 02:08
@not522 not522 added this to the v4.7.0 milestone Jan 14, 2026
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants