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

[FR] Use normal tqdm in notebooks by default #1728

Closed
fehiepsi opened this issue Feb 1, 2019 · 2 comments · Fixed by #1739
Closed

[FR] Use normal tqdm in notebooks by default #1728

fehiepsi opened this issue Feb 1, 2019 · 2 comments · Fixed by #1739

Comments

@fehiepsi
Copy link
Member

fehiepsi commented Feb 1, 2019

Currently, Pyro's MCMC uses tqdm's auto_notebook to display progress bar by default. However, I find that it causes more troubles than advantages:

  • auto_notebook uses jupyter widget, which requires users install widget extension.
  • the widget is only available at running time, which leaves some weird messages when sharing results or reopening it. For example, in this gist, it leaves the line
HBox(children=(IntProgress(value=0, description='Warmup', max=1300, style=ProgressStyle(description_width='ini…

I tried to replace this line by from tqdm import tqdm but it only works for num_chains=1. For multi-chain, the issue tqdm/tqdm#630 still remains.

@neerajprad
Copy link
Member

neerajprad commented Feb 1, 2019

@fehiepsi - I remember I had changed that to tqdm at one point, for exactly this reason. We don't have a good alternative at this point. The following comes to mind:

  • Resort to using tqdm and not auto_notebook when in notebook environment: This gives a really bad output and a long scroll due to the issue that you linked above, and I doubt if there is a resolution for that. I think the tqdm devs will ask us to use auto_notebook instead.
  • Resort to using tqdm but disable progress bar with multiple chains: This seems like throwing the baby out with the bath water kind of a solution. At least currently, the logging stats are available in the notebook environment for users to debug their script. The HBox residue on export, while not ideal, is probably no worse than no output at all.
  • Not use a progress bar at all but just use standard logging in the notebook environment (as we did in the earlier version). This works for the single chain case, but would give garbled results with multiprocessing.
  • Roll out our own version of progress bar (my attempt to do so is in the progbar branch), which I abandoned because while we were able to solve some of these issues, I was creating new problems that tqdm has already solved quite well.
  • Only use auto_notebook for multiple chains in the notebook environment - This seems like a simple work-around, that gives us what we have right now, but preserves the output at least for the single chain case. I can make this change, if this seems acceptable.

I think the best long-term course of option for us is to figure out how to get the above line to render when the notebook is converted into HTML. It doesn't even have to work out of the box, and even if we have to change a few configurations or do it through a script to get it working for our tutorials, that's okay. The users can choose to disable the progress bar before the final export to get rid of the HBox line and just report the diagnostics.

note: Above is my vote for the next step. Feel free to add your suggestion to the list.

@fehiepsi
Copy link
Member Author

fehiepsi commented Feb 1, 2019

@neerajprad About getting HBox line to be rendered in HTML, this PR of nbconvert can be a solution: jupyter/nbconvert#900. I really love the current progressbar messages (the only minor point is when MCMC is so fast in notebook, this issue will happen, but I can solve it by changing jupyter's configs)). Whether it is a normal log or a widget is not important I guess. Because there are many issues for tqdm with multiprocessing in notebooks, I think we can go with your vote and keep the current stage of progressbar for multi-chain. If we can convert ipywidget state to normal log at the end of sampling, then it is the best IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants