Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace fastprogress progress bars with rich #7233

Merged
merged 11 commits into from
Apr 3, 2024

Conversation

fonnesbeck
Copy link
Member

@fonnesbeck fonnesbeck commented Mar 30, 2024

Description

This PR aims to improve the robustness of PyMC's progress bars by implementing them with the rich library in place of fastprogress

Checklist

Type of change

  • New feature / enhancement
  • Bug fix
  • Documentation
  • Maintenance
  • Other (please specify):

📚 Documentation preview 📚: https://pymc--7233.org.readthedocs.build/en/7233/

@fonnesbeck
Copy link
Member Author

Still some testing to do, but they appear to play nicely with Jupyter.

@fonnesbeck fonnesbeck marked this pull request as ready for review April 1, 2024 16:18
@fonnesbeck fonnesbeck requested review from zaxtax and ricardoV94 April 1, 2024 16:19
@fonnesbeck
Copy link
Member Author

I'm sure someone like @zaxtax could be fancier with the progress bar styling. Feel free to embellish!

@fonnesbeck
Copy link
Member Author

fonnesbeck commented Apr 1, 2024

Also, making the progress bars work involved changing how parallel processing works in SMC sampling. Please have a close look.

Copy link

codecov bot commented Apr 1, 2024

Codecov Report

Attention: Patch coverage is 93.56436% with 13 lines in your changes are missing coverage. Please review.

Project coverage is 92.29%. Comparing base (4f6831e) to head (3f107e9).
Report is 5 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7233      +/-   ##
==========================================
- Coverage   92.30%   92.29%   -0.02%     
==========================================
  Files         100      101       +1     
  Lines       16895    16891       -4     
==========================================
- Hits        15595    15589       -6     
- Misses       1300     1302       +2     
Files Coverage Δ
pymc/sampling/mcmc.py 87.71% <100.00%> (ø)
pymc/sampling/parallel.py 88.38% <100.00%> (-0.05%) ⬇️
pymc/smc/sampling.py 99.20% <100.00%> (-0.10%) ⬇️
pymc/stats/log_density.py 98.33% <100.00%> (+0.02%) ⬆️
pymc/util.py 81.09% <100.00%> (+0.16%) ⬆️
pymc/sampling/forward.py 95.94% <94.11%> (+0.03%) ⬆️
pymc/sampling/population.py 73.88% <94.73%> (+0.02%) ⬆️
pymc/tuning/starting.py 91.30% <77.27%> (-0.44%) ⬇️
pymc/variational/inference.py 87.93% <91.42%> (+1.93%) ⬆️

... and 7 files with indirect coverage changes

@ColCarroll
Copy link
Member

Can we haz screenshot plz?

@fonnesbeck
Copy link
Member Author

fonnesbeck commented Apr 1, 2024

pm.sample is quite simple:

image

@fonnesbeck
Copy link
Member Author

pm.fit is similar:

image

@fonnesbeck
Copy link
Member Author

For SMC, since the number of iterations is not fixed I use a spinner for each chain:

image

@zaxtax
Copy link
Contributor

zaxtax commented Apr 1, 2024 via email

@zaxtax
Copy link
Contributor

zaxtax commented Apr 1, 2024 via email

@fonnesbeck
Copy link
Member Author

Yes! Feel free to point me toward relevant examples/resources.

@zaxtax
Copy link
Contributor

zaxtax commented Apr 2, 2024

Yes! Feel free to point me toward relevant examples/resources.

So there are two ways to style. For simple stuff, you can just pass in a styles to track:

import time
from rich.progress import track

for i in track(range(20), description="Processing...", complete_style="rgb(83,145,210)"):
    time.sleep(1) 

But for theming it's probably easiest to take a theme or console object and pass it to Progress

from rich.theme import Theme
custom_theme = Theme({
    "bar.complete": "rgb(83,145,210)",
})

from rich.progress import Progress
from rich.console import Console
import time

with Progress(console=Console(theme=custom_theme)) as progress:
    task1 = progress.add_task("[red]Downloading...", total=100)
    task2 = progress.add_task("[green]Processing...", total=100)
    task3 = progress.add_task("[cyan]Cooking...", total=100)

    while not progress.finished:
        progress.update(task1, advance=0.5)
        progress.update(task2, advance=0.3)
        progress.update(task3, advance=0.9)
        time.sleep(0.02)

The style page (https://rich.readthedocs.io/en/latest/style.html) is the best resource for styling.

@zaxtax
Copy link
Contributor

zaxtax commented Apr 2, 2024

Stylistically, I associate red with error. I prefer blue to indicate normal operation, green for completion, and red for some error state.

@fonnesbeck
Copy link
Member Author

Running:
image

@fonnesbeck
Copy link
Member Author

Complete:
image

@fonnesbeck
Copy link
Member Author

Can now pass custom progressbar themes to samplers!

Copy link
Contributor

@zaxtax zaxtax left a comment

Choose a reason for hiding this comment

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

I don't feel confident in reviewing the sampling.parallel code, but otherwise looks good!

pymc/stats/log_density.py Outdated Show resolved Hide resolved
pymc/sampling/forward.py Outdated Show resolved Hide resolved
@fonnesbeck fonnesbeck requested a review from ricardoV94 April 3, 2024 13:08
@ricardoV94
Copy link
Member

PR looks good to me. Was there any open / known issue with the old library?

@zaxtax
Copy link
Contributor

zaxtax commented Apr 3, 2024 via email

@fonnesbeck
Copy link
Member Author

fonnesbeck commented Apr 3, 2024

These should be more robust progress bars by virtue of being rich text-based. Also the rich project itself appears to be much better-maintained right now.

@ricardoV94
Copy link
Member

mypy is not happy

@Armavica
Copy link
Member

Armavica commented Apr 3, 2024

It looks great on terminal too!
image

@fonnesbeck
Copy link
Member Author

mypy seems to be complaining about things that have nothing to do with this PR

@Armavica
Copy link
Member

Armavica commented Apr 3, 2024

mypy seems to be complaining about things that have nothing to do with this PR

It looks like mcmc.py is the file that causes the error with

pymc/sampling/mcmc.py:1195: error: Argument "progressbar_theme" to "ParallelSampler" has incompatible type "Optional[Theme]"; expected "Theme"

@fonnesbeck fonnesbeck merged commit 22e8f0b into pymc-devs:main Apr 3, 2024
21 checks passed
@fonnesbeck fonnesbeck deleted the rich_progress branch April 3, 2024 18:15
@juanitorduz
Copy link
Contributor

I just say this is a fantastic addition 🙌 ! Thanks!

@jucor
Copy link
Contributor

jucor commented Jul 6, 2024

Adding my thanks here!! 👏 👏 👏 🙌

@fonnesbeck
Copy link
Member Author

Thanks! I'm sure they can be improved, so feel free to suggest additional changes.

@jucor
Copy link
Contributor

jucor commented Jul 6, 2024

I've spotted a bug with the progressbar when PyMC is called from R via Reticulate, and have opened a bug report at both Reticulate and Rich. Mentioned them below in case you want to follow what resolution there might be:

Thanks for the extra option progressbar=False that you have in PyMC, which allows to override, but those progress bars are so useful due to long computations, that I really still need them :)

@jucor
Copy link
Contributor

jucor commented Jul 8, 2024

I have found the root cause for this problem with progress bars! Rstudio Console does not support cursor motion ANSI control sequences. I have opened a feature request upstream in rstudio/rstudio#14942 , as well as an associated bug report rstudio/rstudio#14941 .

Meanwhile, is there a way to have some basic SMC progress bars working without cursor motion control sequences, even if in a much simpler capacity? Basically just printing out the current version of Beta and a timestamp?

mkusnetsov pushed a commit to mkusnetsov/pymc that referenced this pull request Oct 26, 2024
* Replace fastprogress with rich

* Bugfixes for ADVI progress bars

* Bugfixes for MAP progress bars

* Fixed final update to progress bar

* SMC progress bar working

* Fixes to MAP progress bar

* Customize progress bar theme

* Added progressbar_theme argument

* Moved default progressbar theme to util

* Convert compute_log_density to use Progress instead of track

* Getting rid of mypy complaint
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.

7 participants