Skip to content

Use box midpoint to choose finite-difference direction in jacobian_fd#3301

Open
Nas01010101 wants to merge 1 commit into
google-deepmind:mainfrom
Nas01010101:fix/minimize-fd-box-midpoint
Open

Use box midpoint to choose finite-difference direction in jacobian_fd#3301
Nas01010101 wants to merge 1 commit into
google-deepmind:mainfrom
Nas01010101:fix/minimize-fd-box-midpoint

Conversation

@Nas01010101
Copy link
Copy Markdown

Problem

In minimize.py, jacobian_fd chooses each coordinate's finite-difference direction so the perturbation steps away from the nearer bound. It compared x against

mid = 0.5 * (bounds[1] - bounds[0])
eps_vec = np.where(x > mid, -eps, eps)

but 0.5 * (bounds[1] - bounds[0]) is half the box width, not the box midpoint. For bounds that are not centered on the origin this picks the wrong direction. Concretely, with bounds = [10, 20] the comparison value is 5.0 (true midpoint is 15.0), so a point at the lower bound x = 10 satisfies x > 5 and steps −eps, i.e. to 9.9999998 — outside the box.

Fix

Compare against the actual midpoint:

mid = 0.5 * (bounds[0] + bounds[1])

At the lower bound the step is now +eps (inward); at the upper bound −eps (inward).

Tests

Adds test_jacobian_fd_respects_off_center_bounds, which records every point jacobian_fd evaluates with x at the lower bound of the off-center box [10, 20] and asserts they all stay within [lo, hi]. It fails before this change (evaluates 9.9999998) and passes after; the rest of minimize_test.py remains green.

When bounds are provided, `jacobian_fd` chooses each coordinate's
finite-difference direction so the perturbation steps away from the nearer
bound. It compared `x` against `0.5 * (bounds[1] - bounds[0])`, which is half
the box *width*, not the box midpoint. For bounds that are not centered on the
origin this selects the wrong direction, and at the lower bound the perturbation
steps outside the box.

Compare against the midpoint `0.5 * (bounds[0] + bounds[1])` instead. Adds a
regression test checking that all residual evaluations stay within an off-center
box when `x` is at the lower bound; it fails before this change and passes after.
@kevinzakka kevinzakka requested a review from yuvaltassa June 2, 2026 01:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant