Skip to content

update variational_api_quickstart for PyMC 6 - #882

Merged
aloctavodia merged 5 commits into
pymc-devs:mainfrom
YichengYang-Ethan:update/variational-api-quickstart-pymc6
May 26, 2026
Merged

update variational_api_quickstart for PyMC 6#882
aloctavodia merged 5 commits into
pymc-devs:mainfrom
YichengYang-Ethan:update/variational-api-quickstart-pymc6

Conversation

@YichengYang-Ethan

Copy link
Copy Markdown
Contributor

Summary

  • Wrap Approximation.sample(...) calls in with <model>:; PyMC 6 requires an active model context (cells 9, 21, 50, 53). Part of Updates examples for new major releases of pymc/pytensor/arviz #862.
  • Replace removed az.plot_posterior with az.plot_dist(idata, var_names=[...]) (cell 21).
  • Move pm.callbacks.{Tracker,CheckParametersConvergence} references to pm.variational.callbacks (cells 28, 40, 84).
  • Switch total_size=data.shape (tuple) to data.shape[0] (int) — the tuple form hits IndexError in get_scaling (minibatch_rv.py:96) (cell 100).

Verified with Restart & Run All under pymc 6.0.1 / pytensor 3.0.3 / arviz 1.1.0; pre-commit passes.

Part of pymc-devs#862.

Five distinct breakages addressed with minimal source changes; full
Restart & Run All produced fresh outputs and execution metadata.

- Cells 9, 21, 50, 53: Approximation.sample(...) now requires an active
  model context in PyMC 6. Wrapped each call in 'with <model>:'.
- Cell 21: az.plot_posterior was removed in ArviZ 1.0. Replaced with
  az.plot_dist(idata, var_names=[...]), matching the migration pattern
  used in pymc-devs#875.
- Cells 28, 40, 84: pm.callbacks was removed; Tracker and
  CheckParametersConvergence now live in pm.variational.callbacks.
- Cell 100: passing total_size=data.shape (tuple) to a multi-dim
  observed RV triggers an IndexError in get_scaling under
  symbolic_normalizing_constant. Changed to total_size=data.shape[0]
  (int), which is simpler and the intended API regardless.

Verified under pymc 6.0.1, pytensor 3.0.3, arviz 1.1.0. All 105 cells
execute cleanly under Restart & Run All; pre-commit passes.
@review-notebook-app

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@review-notebook-app

review-notebook-app Bot commented May 24, 2026

Copy link
Copy Markdown

View / edit / reply to this conversation on ReviewNB

aloctavodia commented on 2026-05-24T07:12:05Z
----------------------------------------------------------------

No need to use matplotlib inline

Use az.style.use("arviz-variat")


@review-notebook-app

review-notebook-app Bot commented May 24, 2026

Copy link
Copy Markdown

View / edit / reply to this conversation on ReviewNB

aloctavodia commented on 2026-05-24T07:12:06Z
----------------------------------------------------------------

It seems you need ipywidgets


@review-notebook-app

review-notebook-app Bot commented May 24, 2026

Copy link
Copy Markdown

View / edit / reply to this conversation on ReviewNB

aloctavodia commented on 2026-05-24T07:12:07Z
----------------------------------------------------------------

Here you can use plot_dist. You can pass a dictionary where the keys are the names of the models. Check the arviz docs for details


@review-notebook-app

review-notebook-app Bot commented May 24, 2026

Copy link
Copy Markdown

View / edit / reply to this conversation on ReviewNB

aloctavodia commented on 2026-05-24T07:12:07Z
----------------------------------------------------------------

This should be plot_trace_dist. Alternatively we can use plot_rank_dist


@review-notebook-app

review-notebook-app Bot commented May 24, 2026

Copy link
Copy Markdown

View / edit / reply to this conversation on ReviewNB

aloctavodia commented on 2026-05-24T07:12:08Z
----------------------------------------------------------------

Same as previous comment


@review-notebook-app

review-notebook-app Bot commented May 24, 2026

Copy link
Copy Markdown

View / edit / reply to this conversation on ReviewNB

aloctavodia commented on 2026-05-24T07:12:09Z
----------------------------------------------------------------

Idem


@review-notebook-app

review-notebook-app Bot commented May 24, 2026

Copy link
Copy Markdown

View / edit / reply to this conversation on ReviewNB

aloctavodia commented on 2026-05-24T07:12:09Z
----------------------------------------------------------------

Are you always getting the same number?


@review-notebook-app

review-notebook-app Bot commented May 24, 2026

Copy link
Copy Markdown

View / edit / reply to this conversation on ReviewNB

aloctavodia commented on 2026-05-24T07:12:10Z
----------------------------------------------------------------

Kde from seaborn are bad. User arviz


Per @aloctavodia's review:

- Drop %matplotlib inline; apply az.style.use("arviz-variat") (cell 1).
- Install ipywidgets locally; progress-bar warnings cleared from outputs
  (cell 5 thread).
- Replace sns.kdeplot model comparisons with az.plot_dist dict form
  (cells 10, 50, 53).
- az.plot_trace -> az.plot_trace_dist (cell 16). Kept plot_trace_dist
  over plot_rank_dist to stay consistent with the surrounding
  "Above are traces for x^2 and sin(x)" narrative.
- Update markdown around repeated sample_node.eval(): in modern
  PyTensor the random graph is compiled, so repeated .eval() returns
  the cached value. Note this and point to sample_node(size=N) for
  fresh samples (cell 60).
- Replace sns.kdeplot of raw numpy with az.convert_to_dataset +
  az.plot_dist; the first plot also switches to sample_node(size=N)
  so it reflects independent samples (cells 61, 64).
@YichengYang-Ethan

Copy link
Copy Markdown
Contributor Author

@aloctavodia thanks for the thorough review! Addressed all eight comments in the new commit:

  • arviz-variat style applied at the top, no more %matplotlib inline
  • ipywidgets installed locally; widget warnings cleared from outputs
  • Model comparisons (cells 10, 50, 53) now use az.plot_dist({"name": idata, ...}) dict form
  • az.plot_traceaz.plot_trace_dist (kept it over plot_rank_dist to stay consistent with the existing "traces for x² and sin(x)" narrative — happy to switch if you'd prefer rank plots here)
  • Cells 61, 64: az.convert_to_dataset + az.plot_dist; the first one also switches to sample_node(size=N) so the plot reflects independent samples
  • Cell 60 markdown: noted that repeated .eval() on a sample_node returns the cached value in modern PyTensor, and pointed to sample_node(size=N) for fresh samples

Ready for re-review.

@review-notebook-app

review-notebook-app Bot commented May 24, 2026

Copy link
Copy Markdown

View / edit / reply to this conversation on ReviewNB

aloctavodia commented on 2026-05-24T16:43:38Z
----------------------------------------------------------------

Add a legend, pc.add_legend("model") and probably we just want to keep the kde (use visuals to remove elements)


@review-notebook-app

review-notebook-app Bot commented May 24, 2026

Copy link
Copy Markdown

View / edit / reply to this conversation on ReviewNB

aloctavodia commented on 2026-05-24T16:43:39Z
----------------------------------------------------------------

Add semicolon


Per @aloctavodia's followup:

- Cell 10: add legend via pc.add_legend("model") and disable
  credible_interval/point_estimate visuals to keep only the KDE.
- Cell 34: add trailing semicolon to advi.approx to suppress the
  bare-repr output.

Applied the same legend + visuals pattern to the other two
multi-model plot_dist calls (cells 50 and 53) for consistency.
@YichengYang-Ethan

Copy link
Copy Markdown
Contributor Author

Thanks @aloctavodia, applied both:

  • pc.add_legend("model") + visuals dict to keep only the KDE in cell 10
  • semicolon on advi.approx in cell 34

Applied the same legend + visuals pattern to the two other multi-model plot_dist calls (cells 50 and 53) so the look stays consistent — happy to revert if you'd rather only have it on the first one.

CI's black-jupyter wanted the trailing newline removed from the last
line of three code cells; local hooks now match.
PyTensor's compiled function reads the shared RNG state without
advancing it, not because the value is "cached" — the previous
phrasing was inaccurate. Verified by inspecting RNG state across
.eval() calls (state identical) and by compiling an explicit
function with updates={rng_shared: next_rng}, which does advance
and produces fresh samples.
@aloctavodia
aloctavodia merged commit 919cec4 into pymc-devs:main May 26, 2026
2 checks passed
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.

2 participants