Expand dimensionality notebook #5746

ricardoV94 · 2022-05-05T16:22:46Z

I thought it would be useful to expand the dimensionality notebook. It might be a bit too long detailed right now, but I is is probably is easier to discuss from more -> less than the other way around.

I also feel it is better to separate the ellipsis into it's own section and that is not yet finished!

Apologies for tagging so many people for review, but this seems to be a critical part of our documentation that can benefit from varied inputs

Rendered at: https://pymc--5746.org.readthedocs.build/en/5746/learn/core_notebooks/dimensionality.html

review-notebook-app · 2022-05-05T16:22:50Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

docs/source/learn/core_notebooks/dimensionality.ipynb

twiecki · 2022-05-06T10:08:50Z

Please don't force-push as it confuses reviewnb.

ricardoV94 · 2022-05-06T10:10:23Z

Please don't force-push as it confuses reviewnb.

ReviewNB seems useless in this case anyway. I suggest checking the rendered docs directly: https://pymc--5746.org.readthedocs.build/en/5746/learn/core_notebooks/dimensionality.html

docs/source/learn/core_notebooks/dimensionality.ipynb

twiecki · 2022-05-06T10:21:00Z

It's nice for review though.

…

On Fri, May 6, 2022 at 12:10 PM Ricardo Vieira ***@***.***> wrote: Please don't force-push as it confuses reviewnb. ReviewNB seems useless in this case anyway. I suggest checking the rendered docs directly: https://pymc--5746.org.readthedocs.build/en/5746/learn/core_notebooks/dimensionality.html — Reply to this email directly, view it on GitHub <#5746 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFETGHJ7A42XBJWKXDFK5LVITVZTANCNFSM5VFS6AUQ> . You are receiving this because your review was requested.Message ID: ***@***.***>

ricardoV94 · 2022-05-06T10:22:50Z

It's nice for review though.

You can't see the graphviz outputs. But what did force-push screw up?

twiecki · 2022-05-06T10:46:35Z

You can't see the graphviz outputs. But what did force-push screw up?

It didn't allow me to post the comments so I had to copy&paste them over.

OriolAbril · 2022-05-06T22:04:46Z

To the danger of becoming a broken record, I think we should define the intended audience of this notebook so that we are all on the same page and can make meaningful reviews.

For example,

I would also mention that this document is more for advanced users. Most users should be fine with just knowing how dims work and use that as best practice. Ideally we'd have a separate beginner tutorial just on that.

I thought the idea was originally to make a beginner level doc, not really sure now, but I do agree there seems to be a tension here. IMO the scope is quite ambitious and more fitting of advanced level notebook, but the style is more beginner like. i.e. beginners probably don't need to care about all 3 shape, size and dims, 2 at most, and at the same time advanced users don't need to be taught to use size/dims instead of list comprehensions of variables

docs/source/learn/core_notebooks/dimensionality.ipynb

lucianopaz · 2022-05-09T13:55:55Z

docs/source/learn/core_notebooks/dimensionality.ipynb

@@ -9,28 +9,29 @@
   },


We could also mention the difference at the logp level

Reply via ReviewNB

I would not do it here

docs/source/learn/core_notebooks/dimensionality.ipynb

lucianopaz · 2022-05-09T13:55:56Z

docs/source/learn/core_notebooks/dimensionality.ipynb

@@ -9,28 +9,29 @@
   },


One thing that we didn't mention is what coords are used in arviz when the model only defines the dimension name and lets the explicit or implicit size/shape determine the final shape of the RV.

Reply via ReviewNB

I think it's fine to omit that detail from here, as this is not so concerned with the InferenceData machinery

canyon289 · 2022-05-19T13:24:21Z

@ricardoV94 do you need a review from me before the discussion in #5754 is settled or are you good? Want to make sure youre supported :)

ricardoV94 · 2022-05-19T14:21:14Z

@ricardoV94 do you need a review from me before the discussion in #5754 is settled or are you good? Want to make sure youre supported :)

Good for now! Thanks :D

ricardoV94 · 2022-06-14T09:47:02Z

Ready for re-review. I have addressed several comments and removed size explanations for now.

ricardoV94 · 2022-06-14T10:20:10Z

I started but it looked like several comments were not addressed (like typos).

Yeah I forgot to go over the typos, will push again in a sec

ricardoV94 · 2022-06-14T10:45:54Z

Okay, I cleaned and force-pushed for the last time for a while. Check the second commit in ReviewNB to see the differences between now and the original PR commit

ricardoV94 · 2022-06-14T10:48:29Z

Regarding black, the only sensible thing is to add it to the pre-commit. Why haven't we done so already?

ricardoV94 · 2022-06-14T11:00:57Z

ReadTheDocs is erroring out with

Exception occurred:
  File "/home/docs/checkouts/readthedocs.org/user_builds/pymc/conda/5746/lib/python3.8/site-packages/jupyter_client/kernelspec.py", line 294, in get_kernel_spec
    raise NoSuchKernel(kernel_name)
jupyter_client.kernelspec.NoSuchKernel: No such kernel named pymc

michaelosthege · 2022-06-14T11:29:05Z

@ricardoV94 check the source of the *.ipynb at the JSON level. These notebooks sometimes (ß?!) save which kernel they were executed with.

ricardoV94 · 2022-06-14T11:30:01Z

@ricardoV94 check the source of the *.ipynb at the JSON level. These notebooks sometimes (ß?!) save which kernel they were executed with.

Yeah probably something like that

twiecki · 2022-06-14T13:22:13Z

docs/source/learn/core_notebooks/dimensionality.ipynb

@@ -9,231 +9,1402 @@
   },


This is all heading.

Reply via ReviewNB

Yeah, I am not sure why...

Don't you use a ## thing?

This is reviewnb messing up, the rendered version looks right: https://pymcio--5746.org.readthedocs.build/projects/docs/en/5746/learn/core_notebooks/dimensionality.html#id1

canyon289 · 2022-06-14T14:57:17Z

docs/source/learn/core_notebooks/dimensionality.ipynb

@@ -9,231 +9,1402 @@
   },


Are coords not explicit?

Reply via ReviewNB

Coords are explicit, but when you define a distribution you used dims. Or what did you mean?

docs/source/learn/core_notebooks/dimensionality.ipynb

canyon289 · 2022-06-14T14:57:17Z

docs/source/learn/core_notebooks/dimensionality.ipynb

@@ -9,231 +9,1402 @@
   },


Theres all these new words introduced like implcit dimensions, or parameter core dimensions, but theyre not listed at the top. I think its good if they were so its clear what is going to be explained throughout this doc, and provides an easy reference. If they can be linked to the heading in this notebook thatd be even better

Reply via ReviewNB

I think that all the important terms are mentioned in the beginning, and this text clearly states we need to introduce a new concept (core dimensions), just to understand this example. I don't think this concept is as critical to be listed at the top.

OriolAbril

RE rtd failure: we need to do a couple updates to conf.py

https://github.com/ricardoV94/pymc/blob/0e21bb7e928aeab01e8a875ae46616a0650f3beb/docs/source/conf.py#L147-L154

Should be updated to:

# myst config
nb_execution_mode = "force"
nb_kernel_rgx_aliases = {".*": "python3"}
myst_enable_extensions = ["colon_fence", "deflist", "dollarmath", "amsmath", "substitution"]
myst_substitutions = {
    "version_slug": rtd_version,
}
myst_heading_anchors = None

The failure is due to the nb_kernel... one. By default, sphinx executes each notebook with the kernel name listed in the ipynb file which is pymc here, not python3 like the other notebooks. Adding this overrides the kernel provided by the notebook and executes all notebooks with the python3 kernel (the only one we have installed on readthedocs). The other changes are updating/removing deprecated parameters.

docs/source/learn/core_notebooks/dimensionality.ipynb

ricardoV94 · 2022-06-17T15:54:37Z

Ready for re-review

OriolAbril · 2022-06-17T16:18:30Z

The text in all admonitions ends with :::: https://pymcio--5746.org.readthedocs.build/projects/docs/en/5746/learn/core_notebooks/dimensionality.html. Not sure why, but probably related to breaklines. I see the last command removes the ending colon fence which might end up in strange rendering issues at some point.

In admonitions, the text can start in the same line as the opening of the directive, but the closing fence must always be on a different line. The line in which the content is written defines how sphinx interprets that content, see https://sphinx-primer.readthedocs.io/en/latest/core/directives.html

ricardoV94 · 2022-06-17T16:38:05Z

ns, the text can start in the same line as the opening of the directive, but the closing fence must always be on a different line. The line in which the content is written defines how

Fixed it

Other changes: * Seed draws * Give more succinct tittle to last section * Fix directives

ricardoV94 added the docs label May 5, 2022

ricardoV94 requested review from twiecki, lucianopaz, michaelosthege, canyon289 and OriolAbril May 5, 2022 16:22

ricardoV94 force-pushed the dimensionality_notebook_improvement branch from 27b87ad to c025340 Compare May 5, 2022 16:45

ricardoV94 marked this pull request as draft May 5, 2022 16:45

ricardoV94 force-pushed the dimensionality_notebook_improvement branch 2 times, most recently from 0b82f29 to ad20354 Compare May 5, 2022 16:59

ricardoV94 added the request discussion label May 5, 2022

ricardoV94 commented May 6, 2022

View reviewed changes

docs/source/learn/core_notebooks/dimensionality.ipynb Show resolved Hide resolved

twiecki reviewed May 6, 2022

View reviewed changes

ricardoV94 mentioned this pull request May 8, 2022

Remove size from distribution API #5754

Closed

lucianopaz reviewed May 9, 2022

View reviewed changes

ricardoV94 force-pushed the dimensionality_notebook_improvement branch from ad20354 to 8333a1e Compare June 14, 2022 09:46

ricardoV94 requested review from twiecki and lucianopaz June 14, 2022 09:46

ricardoV94 marked this pull request as ready for review June 14, 2022 09:46

ricardoV94 force-pushed the dimensionality_notebook_improvement branch from 8333a1e to c55967e Compare June 14, 2022 09:50

ricardoV94 force-pushed the dimensionality_notebook_improvement branch 2 times, most recently from fdc78ab to 0e21bb7 Compare June 14, 2022 10:44

twiecki reviewed Jun 14, 2022

View reviewed changes

canyon289 reviewed Jun 14, 2022

View reviewed changes

OriolAbril reviewed Jun 14, 2022

View reviewed changes

docs/source/learn/core_notebooks/dimensionality.ipynb Show resolved Hide resolved

ricardoV94 mentioned this pull request Jun 16, 2022

Run black on core notebooks #5901

Merged

ricardoV94 added 2 commits June 17, 2022 17:41

Expand dimensionality notebook

105c141

Update and remove size discussion

2700e86

ricardoV94 force-pushed the dimensionality_notebook_improvement branch 2 times, most recently from c0eed0d to 3748814 Compare June 17, 2022 15:52

ricardoV94 requested review from twiecki, OriolAbril and canyon289 June 17, 2022 15:54

ricardoV94 force-pushed the dimensionality_notebook_improvement branch 2 times, most recently from 474aa67 to c6d97c1 Compare June 17, 2022 16:15

Accommodate feedback and add small section on observed variables

d34dff1

Other changes: * Seed draws * Give more succinct tittle to last section * Fix directives

ricardoV94 force-pushed the dimensionality_notebook_improvement branch from c6d97c1 to d34dff1 Compare June 17, 2022 16:46

twiecki approved these changes Jun 21, 2022

View reviewed changes

twiecki merged commit 0cdd229 into pymc-devs:main Jun 21, 2022

ricardoV94 deleted the dimensionality_notebook_improvement branch June 6, 2023 02:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expand dimensionality notebook #5746

Expand dimensionality notebook #5746

ricardoV94 commented May 5, 2022 •

edited

Loading

review-notebook-app bot commented May 5, 2022

twiecki commented May 6, 2022

ricardoV94 commented May 6, 2022

twiecki commented May 6, 2022 via email

ricardoV94 commented May 6, 2022 •

edited

Loading

twiecki commented May 6, 2022

OriolAbril commented May 6, 2022

lucianopaz May 9, 2022

ricardoV94 Jun 14, 2022

lucianopaz May 9, 2022

ricardoV94 Jun 14, 2022

canyon289 commented May 19, 2022 •

edited

Loading

ricardoV94 commented May 19, 2022

ricardoV94 commented Jun 14, 2022

ricardoV94 commented Jun 14, 2022

ricardoV94 commented Jun 14, 2022 •

edited

Loading

ricardoV94 commented Jun 14, 2022 •

edited

Loading

ricardoV94 commented Jun 14, 2022

michaelosthege commented Jun 14, 2022

ricardoV94 commented Jun 14, 2022

twiecki Jun 14, 2022

ricardoV94 Jun 14, 2022

twiecki Jun 21, 2022

OriolAbril Jun 21, 2022 •

edited

Loading

canyon289 Jun 14, 2022

ricardoV94 Jun 16, 2022

canyon289 Jun 14, 2022

ricardoV94 Jun 16, 2022

OriolAbril left a comment

ricardoV94 commented Jun 17, 2022

OriolAbril commented Jun 17, 2022

ricardoV94 commented Jun 17, 2022

Expand dimensionality notebook #5746

Expand dimensionality notebook #5746

Conversation

ricardoV94 commented May 5, 2022 • edited Loading

review-notebook-app bot commented May 5, 2022

twiecki commented May 6, 2022

ricardoV94 commented May 6, 2022

twiecki commented May 6, 2022 via email

ricardoV94 commented May 6, 2022 • edited Loading

twiecki commented May 6, 2022

OriolAbril commented May 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

canyon289 commented May 19, 2022 • edited Loading

ricardoV94 commented May 19, 2022

ricardoV94 commented Jun 14, 2022

ricardoV94 commented Jun 14, 2022

ricardoV94 commented Jun 14, 2022 • edited Loading

ricardoV94 commented Jun 14, 2022 • edited Loading

ricardoV94 commented Jun 14, 2022

michaelosthege commented Jun 14, 2022

ricardoV94 commented Jun 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OriolAbril Jun 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OriolAbril left a comment

Choose a reason for hiding this comment

ricardoV94 commented Jun 17, 2022

OriolAbril commented Jun 17, 2022

ricardoV94 commented Jun 17, 2022

ricardoV94 commented May 5, 2022 •

edited

Loading

ricardoV94 commented May 6, 2022 •

edited

Loading

canyon289 commented May 19, 2022 •

edited

Loading

ricardoV94 commented Jun 14, 2022 •

edited

Loading

ricardoV94 commented Jun 14, 2022 •

edited

Loading

OriolAbril Jun 21, 2022 •

edited

Loading