-
Notifications
You must be signed in to change notification settings - Fork 227
Modernise README #2575
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
Modernise README #2575
Conversation
Turing.jl documentation for PR #2575 is available at: |
b0af4f4
to
7cbd501
Compare
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.
Greatly improved, thanks! Some discussion points to raise.
I've also requested reviews from @yebai, for obvious reasons, and from @AoifeHughes, because a new comer's eye can be very valuable here. |
I think lots of the text here can go to: https://github.com/TuringLang/.github/edit/main/profile/README.md We could also keep the pointers for related Turing.jl papers. |
README.md
Outdated
|
||
Turing's home page, with links to everything you'll need to use Turing, is available at: | ||
**https://turinglang.org** contains the documentation for the broader TuringLang ecosystem. |
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.
I'd be tempted to change the wording to be a bit "friendlier" and less passive voice
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.
Suggestions are welcome, I don't know how to make "the docs are here" much friendlier.
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.
^^ I think saying "You can find the docs here" is friendlier than " contains docs".
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.
The wording has been changed (twice), it should read better now.
Sure, I was going to work on that too. I think it's worth having info in both though (even if potentially duplicated) because some people will go to github.com/TuringLang and some people will go to github.com/TuringLang/Turing.jl (case in point: if you google for
Will add |
Pull Request Test Coverage Report for Build 16093044479Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2575 +/- ##
===========================================
+ Coverage 58.53% 85.60% +27.06%
===========================================
Files 22 22
Lines 1452 1459 +7
===========================================
+ Hits 850 1249 +399
+ Misses 602 210 -392 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Personally, I’d still like to see quick start guides be part of readme. Ie just tell me the bash to install, run tests etc. I think forcing people to go off the page to find documentation increases likelihood someone just stops |
Done, there's a super simple example in the readme now, let me know what you think. I refrained from adding tests because Turing's tests take forever to run and the average user isn't likely to ever run them (you have to clone the repo anyway, which is not what most people do unless they're hacking on the package itself) |
README.md
Outdated
See [releases](https://github.com/TuringLang/Turing.jl/releases). | ||
julia> @model function my_first_model(data) | ||
mean ~ Normal(0, 1) | ||
sd ~ Exponential(1) |
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.
HalfCauchy
is a better prior than Exponential
or InverseGamma
. See:
On the Half-Cauchy Prior for a Global Scale Parameter
Nicholas G. Polson, James G. Scott
Bayesian Analysis
link
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.
Thanks for the link! Changed to a truncated Cauchy now.
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.
I added a few more comments. The prior on the variance / standard deviation parameter is a bit odd and can be improved. Users tend to pay attention to these examples, so staying with best practices is better.
- Variational inference using [AdvancedVI.jl](https://github.com/TuringLang/AdvancedVI.jl). | ||
- Mode estimation techniques, which rely on SciML's [Optimization.jl interface](https://github.com/SciML/Optimization.jl). | ||
|
||
## Citing Turing.jl |
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.
Could we consider adding a "Papers/Research using TuringJL". Might be useful for SEO reasons + to generally give some extra confidence to new visitors to the page.
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.
I'm going to say no to this, for a couple of reasons:
-
This PR has dragged on for quite long. (Sure, it's partly me being slow, but also there have been a ton of comments on this.) This doesn't strike me as something that's necessary for this PR (i.e., we cannot merge without this), but rather a nice improvement to have. Thus, I would suggest that this be part of a separate PR.
-
There are probably at least a hundred papers that use Turing. I don't want to either list all of them, or pick a handful with the concomitant risk of making it seem like we endorse specific applications.
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.
Couldn't you just put a "cited by" link? https://scholar.google.com/scholar?cites=2146088944996922757&as_sdt=2005&sciodt=0,5&hl=en something like this? I still think it's nice to see on repos
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.
ok done
@yebai, I think I've addressed your comments; are you happy with it in its current state? |
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.
Thanks @penelopeysm -- happy with the changes.
Modernise the README. See the Markdown preview @ https://github.com/TuringLang/Turing.jl/blob/py/readme/README.md
Badges are so 2010s--- I honestly don't think anybody really uses them anymore ever since GitHub Actions took over e.g. travis CI.ReaddedI skipped CI on this PR because this doesn't touch code.
Let me know if there's anything else I should add. Not sure if the last section ('how does it all fit together') should be part of the main docs.