-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Include n_tune, n_draws and t_sampling in SamplerReport #3827
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
Merged
michaelosthege
merged 11 commits into
pymc-devs:master
from
michaelosthege:record-sampling-metadata
Mar 11, 2020
Merged
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
e1844b7
include n_tune, n_draws and t_sampling in SamplerReport
michaelosthege d028ba1
count tune/draw samples instead of trusting parameters
michaelosthege ae9670f
move log info to sampling so number of chains can be included
michaelosthege fc3e18c
add test for SamplerReport n_tune and n_draws
michaelosthege 0e3cd76
use actual number of chains to compute totals
michaelosthege 4b9dc7b
mention new SamplerReport properties
michaelosthege d1c8498
fall back to tune and len(trace) if tune stat is unavailable
michaelosthege 03b21d1
Merge branch 'master' into record-sampling-metadata
michaelosthege 2b191f9
account for differences in stat shape
michaelosthege 2cb5dba
Merge branch 'record-sampling-metadata' of https://github.com/michael…
michaelosthege 49798a2
stop timer before to avoid distortions
michaelosthege File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add implement your suggestions tomorrow - thanks!
As a German, I read
10,000
asfloat(10)
, but maybe we can go with the pythonic neutral ground of10_000
? (According to SI, one should use a thin spaceU+202F
, but in monospaced fonts that's somewhat pointless.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haha! I thought there was a locale-safe version of this. this looks great to me -- feel free to merge.