NotificationsYou must be signed in to change notification settings
Fork2.2k
Star9.4k

Fix progressbar with nested compound step samplers#7776

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.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

jessegrabowski merged 7 commits intopymc-devs:mainfromricardoV94:fix_progress_bar

Jul 16, 2025

Merged

Fix progressbar with nested compound step samplers#7776

jessegrabowski merged 7 commits intopymc-devs:mainfromricardoV94:fix_progress_bar

Jul 16, 2025

Conversation

Copy link

Member

ricardoV94 commentedMay 5, 2025•
edited
Loading

Alternative to#7730 thatfixes#7721 andfixes#7724

ricardoV94 requested review fromCopilot andjessegrabowski

May 5, 2025 09:54

github-actionsbot added the bug label

May 5, 2025

This comment was marked as outdated.

aloctavodia mentioned this pull request

Jun 16, 2025

Add support for multiple BART random variables per model.pymc-devs/pymc-bart#231

Merged

Copy link

Member

aloctavodia commentedJun 21, 2025

Currently, PyMC-BART works without this. However, a recent PR, which allows multiple BART RVs, fails without this. I have not tried with the alternative#7730.

ricardoV94 force-pushed thefix_progress_bar branch from46226a8 to4b4c28dCompare

July 9, 2025 12:08

jessegrabowski requested a review fromCopilot

July 9, 2025 15:23

CopilotAI reviewed

Jul 9, 2025

View reviewed changes

Copy link

CopilotAI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors and centralizes progress bar functionality, updates sampling step methods to use the new API, and adds a regression test for nested compound samplers.

MoveCustomProgress,RecolorOnFailureBarColumn,ProgressBarManager, and related logic frompymc/util.py into a newpymc/progress_bar.py module.
Update all imports to point topymc.progress_bar and adjust references to the new_make_progressbar_update_functions API.
Add an integration test (test_progressbar_nested_compound) to guard against regressions in nestedCompoundStep progress bars.

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/test_progress_bar.py	Add regression test for nested compound-step sampling progress.
pymc/variational/inference.py	Update imports to use new`CustomProgress` location.
pymc/util.py	Remove old progress-bar classes and imports.
pymc/tuning/starting.py	Update progress-bar import to new module.
pymc/step_methods/slicer.py	Rename update-stats API to`_make_progressbar_update_functions`.
pymc/step_methods/metropolis.py	Rename and adjust update-stats API.
pymc/step_methods/hmc/*.py	Rename and adjust update-stats API in HMC and NUTS.
pymc/step_methods/compound.py	Rename and aggregate update-stat functions.
pymc/smc/sampling.py	Update progress-bar import.
pymc/sampling/*.py	Update progress-bar imports in various sampling pathways.
pymc/progress_bar.py	New module consolidating progress-bar logic and classes.
.github/workflows/tests.yml	Include the new test file in CI.

Comments suppressed due to low confidence (1)

pymc/progress_bar.py:37

[nitpick] Consider adding unit tests forCustomProgress (e.g., togglingdisable andinclude_headers) to verify that tasks are added, updated, or suppressed as expected.

class CustomProgress(Progress):

pymc/step_methods/slicer.pyShow resolvedHide resolved

Copy link

codecovbot commentedJul 9, 2025•
edited
Loading

Codecov Report

❌ Patch coverage is95.12195% with10 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.99%. Comparing base (dea3aef) to head (2b50d67).
⚠️ Report is 30 commits behind head on main.

Files with missing lines	Patch %	Lines
pymc/progress_bar.py	93.37%	10 Missing⚠️

Additional details and impacted files

@@            Coverage Diff             @@##             main    #7776      +/-   ##==========================================+ Coverage   89.19%   92.99%   +3.80%==========================================  Files         107      108       +1       Lines       18313    18327      +14     ==========================================+ Hits        16334    17043     +709+ Misses       1979     1284     -695

Files with missing lines	Coverage Δ
pymc/backends/arviz.py	`95.78% <100.00%> (+0.01%)`	⬆️
pymc/logprob/abstract.py	`97.05% <100.00%> (+7.50%)`	⬆️
pymc/sampling/forward.py	`96.75% <100.00%> (+0.01%)`	⬆️
pymc/sampling/mcmc.py	`91.37% <100.00%> (+0.01%)`	⬆️
pymc/sampling/parallel.py	`87.12% <100.00%> (+0.04%)`	⬆️
pymc/sampling/population.py	`70.83% <100.00%> (ø)`
pymc/smc/sampling.py	`99.30% <100.00%> (+<0.01%)`	⬆️
pymc/step_methods/compound.py	`97.87% <100.00%> (+2.51%)`	⬆️
pymc/step_methods/hmc/base_hmc.py	`92.25% <100.00%> (+0.34%)`	⬆️
pymc/step_methods/hmc/hmc.py	`94.59% <100.00%> (+1.04%)`	⬆️
... and7 more

... and18 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jessegrabowski approved these changes

Jul 9, 2025

View reviewed changes

Copy link

Member

jessegrabowski left a comment

Choose a reason for hiding this comment

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

@ricardoV94 can you give the changes i pushed a final once over then we can merge this

Copy link

MemberAuthor

ricardoV94 commentedJul 9, 2025

@aloctavodia can you check if this also fixes the problem on your end?

ricardoV94 commented

Jul 9, 2025

View reviewed changes

pymc/step_methods/metropolis.py OutdatedShow resolvedHide resolved

ricardoV94 marked this pull request as ready for review

July 9, 2025 16:04

jessegrabowski mentioned this pull request

Jul 10, 2025

Fix progress bar error when nestedCompoundStep samplers are assigned#7730

Closed

11 tasks

ricardoV94 force-pushed thefix_progress_bar branch fromc341491 to6748186Compare

July 15, 2025 10:29

Copy link

MemberAuthor

ricardoV94 commentedJul 15, 2025

@jessegrabowski I cleanup a bit the PR, if tests pass I think we're good. Wanna take a last look?

jessegrabowski approved these changes

Jul 15, 2025

View reviewed changes

Copy link

Member

jessegrabowski left a comment

Choose a reason for hiding this comment

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

One docstring out of date, and a small issue I want to bring to your attention.

pymc/progress_bar.py OutdatedShow resolvedHide resolved

pymc/step_methods/hmc/hmc.py OutdatedShow resolvedHide resolved

Copy link

MemberAuthor

ricardoV94 commentedJul 15, 2025

Oops my PR got merged stale and a test is failing

Copy link

MemberAuthor

ricardoV94 commentedJul 15, 2025

The other failure is ours

Copy link

Member

jessegrabowski commentedJul 15, 2025

There was a test with a hardcoded list of expected stats. It should pass now (not sure about the pre-commit problem)

Copy link

MemberAuthor

ricardoV94 commentedJul 15, 2025

pre-commit seems legit

Copy link

Member

aloctavodia commentedJul 16, 2025

I can confirm that this fixes the problem in PyMC-BART

Fix missing import

9f3a119

ricardoV94 force-pushed thefix_progress_bar branch from61c2473 to6ff478aCompare

July 16, 2025 09:57

ricardoV94 requested a review fromjessegrabowski

July 16, 2025 09:57

ricardoV94 added4 commits

July 16, 2025 12:03

Fix progressbar with nested compound step samplers

7164bc6

Move progressbar code to its own module

fd230d3

Renamemake_update_stats_functions to `_make_progressbar_update_fun…

8704886

…ctions`

Cleanup progress_bar

84f92f6

ricardoV94and others added2 commits

July 16, 2025 12:03

Abstract special behavior of NUTS divergences in ProgressBar

5482faf

Every step sampler can now decide whether sampling is failing or not by setting "failing" in the returned update dict

Correctstats_dtypes_shapes isCategoricalGibbsMetropolis

2b50d67

ricardoV94 force-pushed thefix_progress_bar branch from6ff478a to2b50d67Compare

July 16, 2025 10:04

jessegrabowski approved these changes

Jul 16, 2025

View reviewed changes

jessegrabowski merged commit8a436d8 intopymc-devs:main

Jul 16, 2025

25 checks passed

ricardoV94 deleted the fix_progress_bar branch

July 16, 2025 14:06

Armavica mentioned this pull request

Dec 16, 2025

BUG: CategoricalGibbsMetropolis doesn't respect the tune parameter#7997

Open

Labels

bug

Movatterモバイル変換

Fix progressbar with nested compound step samplers#7776

Fix progressbar with nested compound step samplers#7776

Uh oh!

Conversation

ricardoV94 commentedMay 5, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

aloctavodia commentedJun 21, 2025

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

codecovbot commentedJul 9, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jessegrabowski left a comment

Choose a reason for hiding this comment

Uh oh!

ricardoV94 commentedJul 9, 2025

Uh oh!

Uh oh!

ricardoV94 commentedJul 15, 2025

Uh oh!

jessegrabowski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ricardoV94 commentedJul 15, 2025

Uh oh!

ricardoV94 commentedJul 15, 2025

Uh oh!

jessegrabowski commentedJul 15, 2025

Uh oh!

ricardoV94 commentedJul 15, 2025

Uh oh!

aloctavodia commentedJul 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ricardoV94 commentedMay 5, 2025•
edited
Loading

codecovbot commentedJul 9, 2025•
edited
Loading