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.
Reliability and Calibrated Prediction #491
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
Reliability and Calibrated Prediction #491
Changes from 1 commit
71e4a35
8611c5b
fe06eed
2bf64cd
d1854ac
a03ca61
89424d3
e3c3407
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
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.
The fact that was a dataframe wasn't being used anywhere, so I changed that because now below we take advantage of the fact
joint_draws
is an xarray dataset.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.
That's fair.
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 update the code to use xarray-einstats, the inputs are xarray dataset and scalars IIUC, so there should be no need for looping in order to use scipy distributions. I can also try rerunning the notebook and finding other similar instances.
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.
On this one... i think i kind of disagree. While it's not the most efficient way to do this operation, i wanted the parallel with the bootstrapping looping procedures to be clear. We're instantiating in a loop a reasonable model fit to the data in each iteration of the loop, and seeing the variation induced in downstream statistics... i think pedagogically this is simpler than hiding the similarity of the procedure in more efficient code....
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.
Oops I think I misread this comment yesterday, did you want to try and streamline the loop? My only point was that I'd like to keep the analogy with the bootstrap procedure clear....
But I think I could just add more text explaining that too.
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.
Ok, i've added some more text to highlight the relationship between the bootstrap and posterior predictive distributions if you want to work some xarray magic, that'd be cool! Thanks!
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.
Updated, and found a bug in xarray-einstats while doing it 😄 (I have fixed it already)
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.
Is it this one:
I updated to my xarray einstats version, but still getting an error when i try to re-run your new function?
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 recreate with just this call:

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.
Oh!!!! Sorry, i see the issue, the latest change is not on pip!!!! Sorry, slow this morning.
Ehm, that's cool. I'm happy with the changes and you can merge if you want. But for reproducibility should we be concerned that the user can't easily install the required packages?
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 still have one PR I to get into 0.5 release, but I plan to release it soon. We can wait until I actually make the release one of these days, with the watermark having the 0.5.0dev flag though I don't think it is a deal breaker to merge it even before that.
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.
OK, cool! Let's do it. Thanks again for all your help on this one! I'm happy with it now if you are.