Updates to getting started, GLM and model comparison notebooks #5269
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Codecov Report
@@ Coverage Diff @@
## main #5269 +/- ##
==========================================
+ Coverage 79.17% 80.16% +0.98%
==========================================
Files 88 88
Lines 14346 14734 +388
==========================================
+ Hits 11359 11812 +453
+ Misses 2987 2922 -65
|
|
View / edit / reply to this conversation on ReviewNB OriolAbril commented on 2021-12-18T11:11:59Z There are mentions to Thomas being the author both at the top and bottom of the notebook, I would keep it at the bottom with the link to the original blogpost. There are also links to other blogposts in the glm series that point to Thomas' blog, I don't know if those have been converted to examples too or if @twiecki is planning to update the blogposts to v4.
IMO the ideal scenario would be converting those links to links to pymc examples (updated to use v4 sometime soon) and having those notebooks point to the original blogpost. Pointing to the blogposts would be fine if they are updated to use v4 but I would not keep links to the blogposts if they still use v3. This notebook is now part of v4 documentation and should only link to other v4 doc pages and use v4 code (except maybe in the migration guide to compare). fonnesbeck commented on 2021-12-21T16:40:59Z Done |
|
View / edit / reply to this conversation on ReviewNB OriolAbril commented on 2021-12-18T11:12:00Z the mention to glm module needs to be updated. fonnesbeck commented on 2021-12-21T16:29:38Z Removed |
|
View / edit / reply to this conversation on ReviewNB OriolAbril commented on 2021-12-18T11:12:00Z I assume this is due to bambi still being v3 only. @tomicapretto @aloctavodia is development bambi version able to run on v4?
@fonnesbeck proposed not using bambi to circumvent this in slack which could be helpful, but then we'd need to modify the notebook to remove all mentions to easier formula syntax specification for glms which is one of the messages. tomicapretto commented on 2021-12-18T14:51:44Z You're right. Bambi still relies on v3 and it uses Theano.
The development version is not able to run on v4, but I think it is something we could implement without much difficulty. There's only one thing that may be problematic and it is related to the
I plan to make a new release in the first half of January. We could work on porting Bambi development version to v4 after that. fonnesbeck commented on 2021-12-18T16:00:46Z I had theano-pymc installed in the env, which caused this. Goes away when I remove it. twiecki commented on 2021-12-21T08:46:28Z I'd either update the cell or just manually type out that this was on version 4.0.0b1. We update these NB so infrequently that we can expect these warnings to be here for a long time.
OriolAbril commented on 2021-12-21T08:50:52Z going forward, this might still be true for pymc-examples but won't and can't be the case with these notebooks. Moving them back here and doing so with only a handful is to make them part of the versioned docs. Will try to set up CI to autoexecute and everything, but these notebooks need to always be executed with the current version of the documentation. If that can't be for some reason (i.e. too long to run on ci) then they have to be moved back to pymc-examples.
fonnesbeck commented on 2021-12-21T16:38:51Z Should be fixed now. bambi installs theano-pymc as a dependency, which is not ideal, but I assume that goes away in the near future when it is updated to PyMC4/Aesara. |
|
View / edit / reply to this conversation on ReviewNB OriolAbril commented on 2021-12-18T11:12:01Z Use
import sys
instead. There are generally no problems with the fonnesbeck commented on 2021-12-21T16:30:05Z Done |
|
View / edit / reply to this conversation on ReviewNB OriolAbril commented on 2021-12-18T11:12:02Z this function is not needed, we can use fonnesbeck commented on 2021-12-21T16:41:27Z Fixed |
|
View / edit / reply to this conversation on ReviewNB OriolAbril commented on 2021-12-18T11:12:03Z _, ax = plt.subplots(figsize=(7, 7))
y_model = trace.posterior["Intercept"] + trace.posterior["x"] * xr.DataArray(x)
az.plot_lm(
idata=trace,
num_samples=100,
x=x,
y="y",
y_model=y_model,
y_kwargs=dict(marker="x", color="C0"),
y_model_plot_kwargs=dict(lw=0.2, c="k"),
axes=ax,
)
ax.plot(x, true_regression_line, label="true regression line", lw=3.0, c="y")
ax.set_title("Posterior predictive regression lines")
ax.legend(loc=0)
ax.set_xlabel("x")
ax.set_ylabel("y");
|
|
View / edit / reply to this conversation on ReviewNB OriolAbril commented on 2021-12-18T11:12:04Z while we are here, I would use PyMC instead of |
|
You're right. Bambi still relies on v3 and it uses Theano.
The development version is not able to run on v4, but I think it is something we could implement without much difficulty. There's only one thing that may be problematic and it is related to the
I plan to make a new release in the first half of January. We could work on porting Bambi development version to v4 after that. View entire conversation on ReviewNB |
|
I had theano-pymc installed in the env, which caused this. Goes away when I remove it. View entire conversation on ReviewNB |
|
View / edit / reply to this conversation on ReviewNB michaelosthege commented on 2021-12-19T18:43:21Z The introduction is rather outdated:
fonnesbeck commented on 2021-12-21T16:30:33Z I've revised all this now. |
|
View / edit / reply to this conversation on ReviewNB michaelosthege commented on 2021-12-19T18:43:22Z the 2000 is not aligned with the cell below. Just drop it? fonnesbeck commented on 2021-12-21T16:30:49Z Fixed. |
|
View / edit / reply to this conversation on ReviewNB OriolAbril commented on 2021-12-20T00:04:47Z the fonnesbeck commented on 2021-12-21T16:30:55Z Fixed. |
fonnesbeck
changed the title
|
Draft of getting started notebook added. Will finish it up in the next few hours. |
|
could we rename that notebook to I think it is a great notebook for this audience (which imo is what it was aiming for, after all this is based on the paper whose audience is clearly ppl practitioners who keep up with the literature), but it's not really a getting started notebook. New users writing their first bayesian model with a ppl should not need arbitrary logprob terms, distributions nor using aesara explicitly and therefore none of that should be mentioned when writing a tutorial. I think having a "pressing void" on a getting started for pymc newcomers might make it a bit more of a priority. On a related note, maybe we could base the getting started notebook on @AustinRochford webinar for the upcoming pymc-data umbrella series which could then be notebook with a talk. |
|
Completely agree @OriolAbril. I think we want a "Getting started if you know Bayesian modeling" and one if you don't. |
|
I'm all for that, but we should probably not let that be a blocker for getting out of beta. How about we rename this one |
agreed, the blockers should be having everything in the api following the template in #5266 and addressing the TODOs currently visible in the website. @martinacantaro knows better and we'll try to organize when she is back after new year.
that is perfect, we do need to update the toctrees now though so the notebook appears in the website, relevant sidebars and so on, not sure if I'll have time to do this sometime soon.
It should definitely be here I think. The idea was to have a handful of notebooks included in the versioned docs (and therefore always executed with the version of the docs, I'll try to set up CI for this) covering the core functionalities of pymc. |
|
View / edit / reply to this conversation on ReviewNB OriolAbril commented on 2021-12-21T08:44:35Z the target also needs to be updated fonnesbeck commented on 2021-12-21T16:45:53Z Done |
|
View / edit / reply to this conversation on ReviewNB twiecki commented on 2021-12-22T12:44:26Z Why not use the defaults here and generate 1000 samples. |
|
View / edit / reply to this conversation on ReviewNB twiecki commented on 2021-12-22T12:44:27Z Maybe this can be removed as well, no one uses these samplers, and no one should. fonnesbeck commented on 2021-12-24T21:50:18Z I think its important to show that other samplers can be used. There's nothing wrong with the slice sampler. |
|
View / edit / reply to this conversation on ReviewNB twiecki commented on 2021-12-22T12:44:28Z Maybe good opportunity to show how increasing target_accept would help here (if it does)? fonnesbeck commented on 2021-12-24T21:50:37Z Good idea. |
|
I think these could be great getting started guides already. one for if you do not know Bayesian modeling, one if you do. |
|
Its a horseshoe prior. If we chose normal priors we would not be doing sparse regression. View entire conversation on ReviewNB |
|
I can remove it. View entire conversation on ReviewNB |
|
I think its important to show that other samplers can be used. There's nothing wrong with the slice sampler. View entire conversation on ReviewNB |
|
Good idea. View entire conversation on ReviewNB |
|
No, that strips away all the x-axes labels except for the bottom one. View entire conversation on ReviewNB |
|
The plot looks better but it does a poor job with the labels (see newly-pushed version)
View entire conversation on ReviewNB |
|
I can't seem to get any of these working here. I will leave them as markdown for now. View entire conversation on ReviewNB |
|
Oh yeah, it looks like ArviZ sets the grid and ticklables to match the centers of the bins. Changing the order should look better but we should probably add a way to turn this off in ArviZ and have regular grid and ticklabels not tied to any histogram (which doesn't make much sense when comparing) View entire conversation on ReviewNB |
|
wasn't sure it'd work, let's forget about this View entire conversation on ReviewNB |
|
The problem here is that I want the text to say "platform-specific installation guides" and not simply "installation" View entire conversation on ReviewNB |
|
Similar to above, I don't want the text to say "api" I want it to say "API documentation". View entire conversation on ReviewNB |
|
There are some examples of custom text in https://sphinx-primer.readthedocs.io/en/latest/core/roles.html |
|
I've updated these passages View entire conversation on ReviewNB |
|
Done View entire conversation on ReviewNB |
|
done View entire conversation on ReviewNB |
|
Unfortunately swapping the order creates xticklabels that overlap to the point of unreadability. I have left it as is for the time being. View entire conversation on ReviewNB |
|
I think I've addressed all of the comments as best as possible. Let me know if there is anything else that needs attention. |
Will point to the right place and show the custom text. There shouldn't be links hardcoded to v3 docs unless they aim to always point there. |
This updates the GLM and model comparison notebooks to use v4.0 beta.