Address fehiepsi review: remove get_log_density_fn, inline via initialize_model#2
Merged
juanitorduz merged 4 commits intoJun 30, 2026
Conversation
Per fehiepsi's review on PR pyro-ppl#2203, drop the get_log_density_fn helper, its LogDensityInfo return type, and the PositionDict alias; build the log density inline by negating model_info.potential_fn (initialize_model already returns a single-position potential_fn/postprocess_fn). - numpyro/infer/util.py: remove get_log_density_fn + LogDensityInfo; narrow w.message to Warning in find_valid_initial_params so util.py type-checks under ty (newly added to [tool.ty.src]). - numpyro/_typing.py, numpyro/infer/__init__.py, docs/source/utilities.rst: drop the removed symbols. - notebooks/source/other_samplers.ipynb: inline the pattern and re-execute. - test/infer/test_external_helpers.py: inline the pattern; keep constrain_fn and end-to-end MCMCKernel coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
juanitorduz
changed the base branch from
external-samplers
to
external-samplers-master-merge
June 30, 2026 21:43
Reword the markdown lead-in to the Pathfinder log-density cell to say we build the log-density function by negating the potential energy, avoiding the awkward doubled 'negated' phrasing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses fehiepsi's two latest comments on pyro-ppl#2203. Scoped so the diff shows only the review-feedback changes (6 files).
Base branch
Base is `external-samplers-master-merge` = `external-samplers` + a routine merge of `origin/master` (which moved the repo to the ty-only toolchain: mypy removed, `ty.toml` deleted, ty config in `pyproject.toml`; the lone `pyproject.toml` conflict was resolved by keeping master's `[tool.ty.*]` and adding `numpyro/infer/util.py` to `[tool.ty.src].include`). Keeping that merge out of this diff is intentional, so reviewers see just the feedback changes here.
Addressing the review
Both comments asked to drop a thin abstraction; done via full removal + inline:
Verification
🤖 Generated with Claude Code