-
-
Notifications
You must be signed in to change notification settings - Fork 253
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
[WIP] Review changes to survival_analysis.ipynb #153
Conversation
…ergence part (looking into it).
…the convergence (looking into it this week)
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
@MarcoGorelli, @OriolAbril Regarding the convergence issue, increasing the target_accept does the trick: the number divergences is cut in half, r_hat is 1, the sampler converges, the chains in the forest plot lay snug, and the posterior looks almost exactly the same as in the original notebook. Is that enough for the job at hand or should we reparameterize? |
View / edit / reply to this conversation on ReviewNB OriolAbril commented on 2021-05-05T20:50:16Z As Marco pointed out recently and added to the style guide, we should specify which error to catch, my original advise was not best practices. Here is the link: https://github.com/pymc-devs/pymc3/wiki/PyMC3-Jupyter-Notebook-Style-Guide |
View / edit / reply to this conversation on ReviewNB OriolAbril commented on 2021-05-05T20:50:17Z I would delete the commented line (if you haven't 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.
Increasing the target_accept is more than enough for our purposes here. I commented two very minor nits, it could be merged as is once you push the converged results too.
This is already great, thanks for the PR! I hope it has been an enriching and pleasurable experience
…o the minor nits.
Yes, enriching and pleasurable, thank you! Contributing is definitely one of the best way to learn, and with your help, support, and patience this was an invaluable experience. I’ll be on the lookout for opportunities to continue participating :) |
Curious, when do the notebooks / code base update to reflect the merged changes? What does it depend on? Thank you. |
Great job @olgadk7 ! The website'll be updated with the next release of PyMC3 (ATM it's not updated after each commit to this repo - but perhaps it should be!) |
And this is a completely manual process which we sometimes forget or do significantly later than the release. 😅
Indeed, it would be amazing to have both release and dev version docs, with the dev version being updated with every commit here, I believe we already have an issue for this. |
Wanted to submit the incorporated changes recommended in #96: all but the convergence one (looking into it this week). Had to start another branch as the original one had irresolvable conflicts (guessing because of me not submitting often).
Closes pymc-devs/pymc#4603, addresses #128