CU-5t5y0p Regarding issue #222 of pymc-devs/pymc-examples #4986
Conversation
…theorem, Markov Chain (MC) and Markov Chain Monte Carlo (MCMC)
There was a problem hiding this comment.
Thanks for the PR! Haven't had time to read the definitions yet but I wanted to do a first review round as soon as possible.
To not repeat myself too much, also read #4984 (review) as I think links between terms will also be helful here and there aren't any yet
Codecov Report
@@ Coverage Diff @@
## main #4986 +/- ##
==========================================
- Coverage 76.56% 76.34% -0.22%
==========================================
Files 86 86
Lines 13965 13931 -34
==========================================
- Hits 10692 10636 -56
- Misses 3273 3295 +22
|
There was a problem hiding this comment.
Thanks for the quick update, I meant links between terms, like the suggestion I did in the other PR. I have added a suggestion of link between terms, I hope it's clear enough
References to external definitions are also fine but a different thing altogether. I don't know most of these websites, but we'd need some guarantees that they are trustworthy and stable (i.e. that these urls won't have changed 4 years from now).
Note also that some links are not correctly formatted, you have to make sure to follow markdown syntax. If you go over the checks at the bottom of this PR, you'll see a readthedocs check. Clicking on the "details" there will show you a preview of the docs with the changes in your PR. Here is how they look right now: https://pymc3--4986.org.readthedocs.build/en/4986/glossary.html?highlight=glossary
I would also remove the acronyms from the term name and include it only in the description so it's easier to reference those terms. If some acronym is especially important by itself (i.e. MCMC appears in half of the pages of the documentation), then make the term have two "names" like with myst and myst markdown in https://github.com/executablebooks/jupyter-book/blob/master/docs/reference/glossary.md
docs/source/glossary.md
Outdated
| A Markov chain or Markov process is a stochastic model describing a sequence of possible events in which the probability of each event depends only on the state attained in the previous event. For a visual explantation, visit the link. | ||
|
|
||
| [Markov Chain Monte Carlo](https://en.wikipedia.org/wiki/Markov_chain_Monte_Carlo) | ||
| [MCMC](https://machinelearningmastery.com/markov-chain-monte-carlo-for-probability/) |
There was a problem hiding this comment.
Let's leave only one title (and one title link) for Markov Chain Monte Carlo, which can look like this:
[Markov Chain Monte Carlo (MCMC)]
There was a problem hiding this comment.
I prefer very much to have the two titles, especially thinking about linking to that term, but I agree both should have the same link.
If we use Markov Chain Monte Carlo (MCMC) we have to use
{term}`Markov Chain Monte Carlo (MCMC)`
to link to it. Otherwise we should be able to use both
{term}`MCMC`
{term}`Markov Chain Monte Carlo`
or maybe only one of them, not completely sure how it works, if both are usable, only the first, only the last... but I do know the glossary allows multiple names per definition.
|
Could someone maybe check my commits? |
There was a problem hiding this comment.
remember that you can check the preview of the glossary on the PR checks section, the rtd one which is the only one relevant here runs always
I have also marked some previous discussions as resolved, but not all of them, so don't directly disregard the messages above
docs/source/glossary.md
Outdated
| [Bayes' theorem](https://en.wikipedia.org/wiki/Bayes%27_theorem) | ||
| Describes the probability of an event, based on prior knowledge of conditions that might be related to the event. For example, if the risk of developing health problems is known to increase with age, Bayes' theorem allows the risk to an individual of a known age to be assessed more accurately (by conditioning it on their age) than simply assuming that the individual is typical of the population as a whole. | ||
| Formula: | ||
| {term}`P(A|B) = (P(B|A) P(A))/P(B)` |
There was a problem hiding this comment.
This will not work, term is only to link to terms defined in the glossary. Math can be inserted either as inline equation or block equation with dollar delimiters like one would do on latex/jupyter notebook (reference: https://myst-parser.readthedocs.io/en/latest/syntax/optional.html#dollar-delimited-math, we already have everything configured for the repo, so pay attention only to the examples)
…equation in Bayes Theorem; Made 2 headings for MCMC
|
I am going to have to close this PR. For some reason, I had to create a new forked repo and thus a new PR for that repo. @OriolAbril could you maybe create a website link for that PR? so that I can see what impact my changes would have on the website? My apologies for the inconvenience. Link to the PR -> #5014 |
In docs/source/glossary.md
Added definition for Equidispersion, Generalized Poisson PMF, Bayes' Theorem, MC, MCMC