Include n_tune, n_draws and t_sampling in SamplerReport #3827
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3827 +/- ##
==========================================
+ Coverage 90.77% 90.79% +0.01%
==========================================
Files 135 135
Lines 21074 21113 +39
==========================================
+ Hits 19130 19169 +39
Misses 1944 1944
|
pymc3/backends/report.py
Outdated
| @@ -151,7 +175,8 @@ def _add_warnings(self, warnings, chain=None): | |||
| warn_list.extend(warnings) | |||
|
|
|||
| def _log_summary(self): | |||
|
|
|||
| if self._n_tune is not None and self._n_draws is not None and self._t_sampling is not None: | |||
| logger.info(f'Sampling {self.n_tune} tune and {self.n_draws} draw iterations took {self.t_sampling:.0f} seconds.') | |||
There was a problem hiding this comment.
Can this include the number of chains and total number of draws? That might clarify how we count: For 1000 draws in 4 chains, the progressbar will go to 4000, so a message like "Sampling 500 tune and 1000 draws in 4 chains (2000 plus 4000 draws total) took 18 seconds." might be helpful.
What do you think?
Also, I might use {self.n_tune:,d} and {self.n_draws:,d} to get commas in the number, but that's a very weak desire.
There was a problem hiding this comment.
I'll add implement your suggestions tomorrow - thanks!
As a German, I read 10,000 as float(10), but maybe we can go with the pythonic neutral ground of 10_000? (According to SI, one should use a thin space U+202F, but in monospaced fonts that's somewhat pointless.)
There was a problem hiding this comment.
This is with the update I just pushed:
5 chains sequentially, interrupted during tuning of the first:
Interrupted during the second chain:
Not interrupted:
There was a problem hiding this comment.
haha! I thought there was a locale-safe version of this. this looks great to me -- feel free to merge.
(because of KeyboardInterrupt)
+ clarify that n_tune are not necessarily in the trace
+ CompoundStep causes 2D stats + fix calculation without stats
…osthege/pymc3 into record-sampling-metadata
|
Finally. This took embarrassingly many commits to go green. |
This PR adds three new (optional) properties to
SamplerReport:n_tune: Number of tune iterations.n_draws: Number of draw iterations.t_sampling: Number of seconds that the sampling procedure took. (Includes parallelization overhead.)While
n_drawsmay be retrieved from the trace, information aboutn_tuneis lost unlessdiscard_tuned_samples=True. However, ArviZ currently does not look at the"tune"sampler stat, making it very inconvenient to keep tuning iterations and use ArviZ at the same time.The
t_sampling(wall-clock) time is not kept automatically, but super useful for comparing sampler efficiency. We could also use it directly in the benchmarks..I've included it in the
_log_summaryatINFOlevel:What do you think?