Use fastprogress instead of tqdm progressbar #3693
Conversation
|
oops I just realized, fastprogressbar requires at least Python 3.6 . It seems that the next numpy release will requiere python 3.6+, thus maybe is ok for us to also require Python 3.6+ in the near future. I guess we should do a a release before dropping 3.5 |
Codecov Report
@@ Coverage Diff @@
## master #3693 +/- ##
===========================================
+ Coverage 62.46% 89.94% +27.47%
===========================================
Files 134 134
Lines 20194 20413 +219
===========================================
+ Hits 12615 18361 +5746
+ Misses 7579 2052 -5527
|
|
Need to add EDIT: never mind, need to repair our requirements files to make them complementary. |
|
LGTM |
pymc3/parallel_sampling.py
Outdated
| @@ -346,8 +348,7 @@ def __init__( | |||
| start_chain_num=0, | |||
| progressbar=True, | |||
| ): | |||
| if progressbar: | |||
| from tqdm import tqdm | |||
| from fastprogress import progress_bar | |||
There was a problem hiding this comment.
i think this can go to the top of the file. if i recall, it was here and protected because of install difficulties with tqdm.
If it stays here, I would leave it in a if progressbar: statement.
| desc=self._desc.format(self) | ||
| ) | ||
| self._progress = progress_bar( | ||
| range(chains * (draws + tune)), display=progressbar, auto_update=False |
There was a problem hiding this comment.
oooh, this display=progressbar is fancy! I would move the import to the top for sure, then!
pymc3/variational/inference.py
Outdated
| 'or Inference instance' % | ||
| set(_select.keys())) | ||
| raise KeyError( | ||
| "method should be one of %s " "or Inference instance" % set(_select.keys()) |
There was a problem hiding this comment.
black did a bad job folding this, I think
| "method should be one of %s " "or Inference instance" % set(_select.keys()) | |
| "method should be one of %s or Inference instance" % set(_select.keys()) |
pymc3/variational/inference.py
Outdated
| raise TypeError('method should be one of %s ' | ||
| 'or Inference instance' % | ||
| set(_select.keys())) | ||
| raise TypeError("method should be one of %s " "or Inference instance" % set(_select.keys())) |
There was a problem hiding this comment.
| raise TypeError("method should be one of %s " "or Inference instance" % set(_select.keys())) | |
| raise TypeError("method should be one of %s or Inference instance" % set(_select.keys())) |
| @@ -791,10 +807,12 @@ def fit( | |||
| inference = _select[method](model=model, **inf_kwargs) | |||
| else: | |||
| raise KeyError( | |||
| "method should be one of %s " "or Inference instance" % set(_select.keys()) | |||
| f"method should be one of {set(_select.keys())} or Inference instance" | |||
|
It would be nice to see #iterations per second (currently there is only information about logp (I checked it with find_MAP)). In this sense previous progressbar was more informative Also it does not show the expected time, just 0:00<0:00 |
|
@aakhmetz I agree, fastprogress is a simpler progress bar, but I have found it to be more robust than tqdm, which was my original motivation for making the switch. |
|
I have an ongoing project where I fit a large system of ODEs to the data and where it is desirable to see the current speed for performing iterations. Please excuse me for a small objection, you definitely foresee much further than me |
|
Fair point. You might have a peek at the |
Use fastprogress instead of tqdm progressbar (pymc-devs#3693)
Fix problems with #3667