-
Couldn't load subscription status.
- Fork 106
Turing 0.41. #661
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
Turing 0.41. #661
Conversation
|
Preview the changes: https://turinglang.org/docs/pr-previews/661 |
| w = [0.5, 0.5] | ||
| μ = [-3.5, 0.5] | ||
| mixturemodel = MixtureModel([MvNormal(Fill(μₖ, 2), I) for μₖ in μ], w) | ||
| μ = [-2.0, 2.0] | ||
| mixturemodel = MixtureModel([MvNormal(Fill(μₖ, 2), 0.2 * I) for μₖ in μ], w) | ||
| # We draw the data points. | ||
| N = 60 | ||
| N = 30 | ||
| x = rand(mixturemodel, N); |
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.
Having 30 observations just makes life easier because it triggers fewer resampling steps with PG. To compensate for that I made the data more tightly clustered to make sure that the results are still somewhat meaningful.
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.
locally, this cuts the time taken to run this tutorial to about 12 minutes. I'm not sure what it was before but I think it was certainly over 20 minutes.
| model = linear_regression(train, train_target) | ||
| chain = sample(model, NUTS(), 5_000) | ||
| chain = sample(StableRNG(468), model, NUTS(), 20_000) | ||
| ``` |
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 don't actually know what went wrong here, i struggled to get this to work without bumping the samples. It might just be that the 5000 samples one just happened to be a very lucky seed. It takes very little time because NUTS is quite fast anyway.
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 assume the OLS/Bayesian linear regression match previously was just a fluke. In that case, happy to approve, thanks for doing this.
| @assert isapprox(bayes_train_loss, ols_train_loss; rtol=0.01) "Difference between Bayesian training loss ($bayes_train_loss) and OLS training loss ($ols_train_loss) unexpectedly large!" | ||
| @assert isapprox(bayes_test_loss, ols_test_loss; rtol=0.05) "Difference between Bayesian test loss ($bayes_test_loss) and OLS test loss ($ols_test_loss) unexpectedly large!" | ||
| @assert bayes_train_loss > ols_train_loss "Bayesian training loss ($bayes_train_loss) <= OLS training loss ($bayes_train_loss)" | ||
| @assert bayes_test_loss < ols_test_loss "Bayesian test loss ($bayes_test_loss) >= OLS test loss ($ols_test_loss)" |
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.
Do you know if it was just happenstance that these actually pretty much matched each other before?
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.
Err, not sure to be honest. I think the atols were quite fragile, even changing the way the test/train split was done led to things failing there. So I'd say yes.
I think pretty much all that is needed is fixing
initial_params.Edit: Oh, there was
istrans->is_transformedtoo.Edit 2: Oh no, there's SamplingContext 😭