Skip to content

Make optimize interface more aligned with optimize_iterative - #65

Merged
adrn merged 4 commits into
mainfrom
optimize-interface-symmetry
Aug 19, 2026
Merged

adrn merged 4 commits into
mainfrom
optimize-interface-symmetry

Conversation

@adrn

@adrn adrn commented Aug 19, 2026

Copy link
Copy Markdown
Owner

This adds initial_params (which routes to numpyro's init_loc_fn) and progress as args, like optimize_iterative has.

adrn and others added 4 commits August 19, 2026 19:39
optimize_iterative already took both. optimize took neither: starting from
known parameters meant hand-rolling numpyro's init machinery, and silencing
the progress bar meant knowing it lived under svi_run_kwargs. The recipe was
spelled out in optimize's own docstring, which is the tell that it wanted to
be a keyword argument instead.

Both stay backward compatible. init_loc_fn remains as the escape hatch for
the strategies initial_params cannot express, svi_run_kwargs remains, and an
explicit svi_run_kwargs["progress_bar"] still wins over progress=.

initial_params assigns init_loc_fn rather than branching later, so the
existing already-constructed-guide check covers it for free.

Also corrects a false claim in the init_loc_fn docstring: a partial dict does
not leave unlisted sites at the guide's default. init_to_value replaces the
strategy wholesale and defers those sites to init_to_uniform(radius=2) in
unconstrained space, so AutoDelta's init_to_median never runs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Renames TestOptimizeInitLocFn to TestOptimizeStartingPoint, since it now
covers both spellings, and pins the new behavior: the round trip through
pack_numpyro_pars, the initial_params/init_loc_fn conflict, the
already-constructed-guide guard, and that a value for a site fixed_pars has
conditioned out is harmless (optimize_iterative leans on that too).

test_partial_values_fall_back_to_the_default asserted a falsehood in its name
and docstring, and its assertions passed either way, so nothing was being
caught. Rewritten as test_partial_initial_params_randomizes_the_rest, which
discriminates: the latents prior is Normal(0, 1), so a median init would put
them all at exactly zero and fail the spread check.

An empty dict is treated as "not given" rather than silently switching every
site onto init_to_uniform, which is why the check is truthiness and not
`is not None`. There is a test for that, since it looks like a typo otherwise.

No test for progress= itself: it is a one-line dict merge, and asserting on
tqdm's stderr would be flaky for no coverage gain. Its precedence branch is
guarded by test_transforms, which now says why it keeps using svi_run_kwargs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
These run 10k and 50k SVI steps, and drew a bar through the whole suite.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six call sites across four notebooks, including one inside a fit_kwargs dict
literal and one commented-out call kept next to a live one.

Behavior is unchanged -- progress_bar was already False at every one of them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@adrn
adrn enabled auto-merge August 19, 2026 23:43
@adrn

adrn commented Aug 19, 2026

Copy link
Copy Markdown
Owner Author

Docs failure here is an intermittent issue...

@adrn
adrn disabled auto-merge August 19, 2026 23:54
@adrn
adrn merged commit 28856a0 into main Aug 19, 2026
15 of 16 checks passed
@adrn
adrn deleted the optimize-interface-symmetry branch August 19, 2026 23:54
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