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.

Sign up for GitHub

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

Cleanup pypesto/profile/profile_next_guess.py #1167

Merged
merged 1 commit into from
Oct 31, 2023
Merged

Cleanup pypesto/profile/profile_next_guess.py #1167

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 47 additions & 69 deletions pypesto/profile/profile_next_guess.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,15 @@ def next_guess(
return fixed_step(
x, par_index, par_direction, profile_options, problem
)
elif update_type == 'adaptive_step_order_0':
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know that it technically makes no difference, but elif would highlight more, that these comparisons belong to each other wouldn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do they really belong together? I'd say no. Fixed step: we are good to go. Adaptive step; we need to set order. I think this makes it clearer.

Copy link
Collaborator

Choose a reason for hiding this comment

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

one can argue like that as well. No strong preference here 👍🏼


if update_type == 'adaptive_step_order_0':
order = 0
elif update_type == 'adaptive_step_order_1':
order = 1
elif update_type == 'adaptive_step_regression':
order = np.nan
else:
raise Exception(
raise ValueError(
f'Unsupported `update_type` {update_type} for `next_guess`.'
)

Expand Down Expand Up @@ -207,14 +208,12 @@ def clip_to_bounds(step_proposal):
# check whether we must make a minimum step anyway, since we're close to
# the next bound
min_delta_x = x[par_index] + par_direction * options.min_step_size
if par_direction == -1:
if min_delta_x < problem.lb_full[par_index]:
step_length = problem.lb_full[par_index] - x[par_index]
return x + step_length * delta_x_dir
else:
if min_delta_x > problem.ub_full[par_index]:
step_length = problem.ub_full[par_index] - x[par_index]
return x + step_length * delta_x_dir
if par_direction == -1 and (min_delta_x < problem.lb_full[par_index]):
step_length = problem.lb_full[par_index] - x[par_index]
return x + step_length * delta_x_dir
elif par_direction == 1 and (min_delta_x > problem.ub_full[par_index]):
step_length = problem.ub_full[par_index] - x[par_index]
return x + step_length * delta_x_dir

# parameter extrapolation function
def par_extrapol(step_length):
Expand Down Expand Up @@ -263,37 +262,19 @@ def par_extrapol(step_length):
next_obj = problem.objective(problem.get_reduced_vector(next_x))

# iterate until good step size is found
if next_obj_target < next_obj:
# The step is rather too long
return do_line_search(
next_x,
step_size_guess,
'decrease',
par_extrapol,
next_obj,
next_obj_target,
clip_to_minmax,
clip_to_bounds,
par_index,
problem,
options,
)

else:
# The step is rather too short
return do_line_search(
next_x,
step_size_guess,
'increase',
par_extrapol,
next_obj,
next_obj_target,
clip_to_minmax,
clip_to_bounds,
par_index,
problem,
options,
)
PaulJonasJost marked this conversation as resolved.
Show resolved Hide resolved
return do_line_search(
next_x,
step_size_guess,
"decrease" if next_obj_target < next_obj else "increase",
par_extrapol,
next_obj,
next_obj_target,
clip_to_minmax,
clip_to_bounds,
par_index,
problem,
options,
)


def handle_profile_history(
Expand Down Expand Up @@ -336,10 +317,6 @@ def handle_profile_history(
last_delta_x = (
current_profile.x_path[:, -1] - current_profile.x_path[:, -2]
)
step_size_guess = np.abs(
current_profile.x_path[par_index, -1]
- current_profile.x_path[par_index, -2]
)
delta_x_dir = last_delta_x / step_size_guess
elif np.isnan(order):
# compute the regression polynomial for parameter extrapolation
Expand Down Expand Up @@ -372,7 +349,7 @@ def get_reg_polynomial(
for i_par in range(problem.dim_full):
if i_par in problem.x_fixed_indices:
# if we meet the current profiling parameter or a fixed parameter,
# there is nothing to do, so pass an np.nan
# there is nothing to do, so pass a np.nan
reg_par.append(np.nan)
else:
# Do polynomial interpolation of profile path
Expand Down Expand Up @@ -403,7 +380,7 @@ def get_reg_polynomial(
def do_line_search(
next_x: np.ndarray,
step_size_guess: float,
direction: str,
direction: Literal['increase', 'decrease'],
par_extrapol: Callable,
next_obj: float,
next_obj_target: float,
Expand Down Expand Up @@ -443,21 +420,19 @@ def do_line_search(

if hit_bounds:
return next_x
else:
# compute new objective value
problem.fix_parameters(par_index, next_x[par_index])
last_obj = copy.copy(next_obj)
next_obj = problem.objective(problem.get_reduced_vector(next_x))

# check for root crossing and compute correct step size in case
if direction == 'decrease' and next_obj_target >= next_obj:
return next_x_interpolate(
next_obj, last_obj, next_x, last_x, next_obj_target
)
elif direction == 'increase' and next_obj_target <= next_obj:
return next_x_interpolate(
next_obj, last_obj, next_x, last_x, next_obj_target
)

# compute new objective value
problem.fix_parameters(par_index, next_x[par_index])
last_obj = copy.copy(next_obj)
next_obj = problem.objective(problem.get_reduced_vector(next_x))

# check for root crossing and compute correct step size in case
if (direction == 'decrease' and next_obj_target >= next_obj) or (
direction == 'increase' and next_obj_target <= next_obj
):
return next_x_interpolate(
next_obj, last_obj, next_x, last_x, next_obj_target
)


def next_x_interpolate(
Expand All @@ -480,12 +455,15 @@ def clip(
lower: Union[float, np.ndarray],
upper: Union[float, np.ndarray],
) -> Union[float, np.ndarray]:
"""Restrict a scalar or a vector to given bounds."""
"""Restrict a scalar or a vector to given bounds.

``vector_guess`` is modified in-place if it is an array.
"""
if isinstance(vector_guess, float):
vector_guess = np.max([np.min([vector_guess, upper]), lower])
else:
for i_par, i_guess in enumerate(vector_guess):
vector_guess[i_par] = np.max(
[np.min([i_guess, upper[i_par]]), lower[i_par]]
)
return np.max([np.min([vector_guess, upper]), lower])

for i_par, i_guess in enumerate(vector_guess):
vector_guess[i_par] = np.max(
[np.min([i_guess, upper[i_par]]), lower[i_par]]
)
return vector_guess
Loading