Skip to content
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

Docs proofread #339

Merged
merged 7 commits into from
Nov 17, 2023
Merged

Docs proofread #339

merged 7 commits into from
Nov 17, 2023

Conversation

nspope
Copy link
Contributor

@nspope nspope commented Nov 16, 2023

Some suggested changes to the docs wrt the variational gamma method.

@nspope
Copy link
Contributor Author

nspope commented Nov 16, 2023

Only change to code is to import tsdate.priors.parameter_grid as tsdate.build_parameter_grid to expose an analogue for tsdate.build_prior_grid

@nspope
Copy link
Contributor Author

nspope commented Nov 16, 2023

One comment: the phrase "loopy belief propagation" as used here

suffers from the theoretical problem of "loopy belief propagation". Occasionally

doesn't make much sense to me-- I've always seen LBP used to refer to a method (apply belief propagation repeatedly to graphs with loops) rather than as a "theoretical problem", per se. So my vote is to remove references to LBP from methods.md (I don't think they will be meaningful to most readers, anyway).

@nspope
Copy link
Contributor Author

nspope commented Nov 16, 2023

I'm having issues building the docs locally, so haven't yet checked to see if the links all work.

@hyanwong
Copy link
Member

@Mergifyio rebase

Copy link

mergify bot commented Nov 17, 2023

rebase

☑️ Nothing to do

  • -conflict [:pushpin: rebase requirement]
  • -closed [:pushpin: rebase requirement]

@hyanwong
Copy link
Member

One comment: the phrase "loopy belief propagation" as used here

suffers from the theoretical problem of "loopy belief propagation". Occasionally

doesn't make much sense to me-- I've always seen LBP used to refer to a method (apply belief propagation repeatedly to graphs with loops) rather than as a "theoretical problem", per se. So my vote is to remove references to LBP from methods.md (I don't think they will be meaningful to most readers, anyway).

Ah, good point about the terminology. I will try to reword. I think we should make the point that the previous methods (without iterative convergence) were not exact, however.

@hyanwong
Copy link
Member

I'll merge this and fix any refs plus address the LBP stuff in a follow-up PR

@hyanwong hyanwong merged commit e24af53 into tskit-dev:main Nov 17, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants