search

TuringLang / docs

Documentation and tutorials for the Turing language
https://turinglang.org/docs/tutorials/docs-00-getting-started/
MIT License
225 stars 97 forks source link

Yongchao/update variational inference tutorial #349

Closed YongchaoHuang closed 1 year ago

yebai commented 1 year ago

@YongchaoHuang can you take a look at why CI (buildkite, see above) is broken for this PR, please?

YongchaoHuang commented 1 year ago

@YongchaoHuang can you take a look at why CI (buildkite, see above) is broken for this PR, please?

sure, I will have an investigation later.

YongchaoHuang commented 1 year ago

@YongchaoHuang can you take a look at why CI (buildkite, see above) is broken for this PR, please?

Got it: I modified the unstandardize() function - the std and mean of an DataFrame raise the error. It's fixed by converting Matrix(DataFrame) or Array(DataFrame) .

yebai commented 1 year ago

@YongchaoHuang I fixed all the formatting issues by accepting all the suggested changes by JuliaFormatter.

YongchaoHuang commented 1 year ago

@torfjelde @yebai your review on these changes would be very much appreciated.

yebai commented 1 year ago

@YongchaoHuang buildkite CI is not passing, can you take a look please?

YongchaoHuang commented 1 year ago

@YongchaoHuang buildkite CI is not passing, can you take a look please?

Yes, thanks Hong. I am looking into it.

YongchaoHuang commented 1 year ago

@YongchaoHuang buildkite CI is not passing, can you take a look please?

Yes, thanks Hong. I am looking into it.

The Buildkite error says 'ERROR: LoadError: UndefVarError: histogram not defined'. histogram plot is from Plots, which is included already. It worked on my local machine. I just replaced histogram by Plots.histogram. Also included using Distributions. Let's see how it works.

YongchaoHuang commented 1 year ago

@YongchaoHuang buildkite CI is not passing, can you take a look please?

Yes, thanks Hong. I am looking into it.

@yebai passed now.

YongchaoHuang commented 1 year ago

Looks good overall! But I would like to resolve the namespace issue before we merge. There are also a couple of redundant changes that can be reverted.

@torfjelde Thanks a lot for your time and suggestions. I will follow each question and respond today.

YongchaoHuang commented 1 year ago

@devmotion Thanks a lot for your help in improving this PR, the build is successful (see here). Maybe ready to be merged?

YongchaoHuang commented 1 year ago

@devmotion I applied most (if not all) of the changes suggested, but get messed up when replying to your suggestions on web and VScode, and some replies are not shown (now sure why). The most recent commit d221e21 should have all changes applied. Ready to merge? Thanks.

YongchaoHuang commented 1 year ago

Looks good, I pushed a final commit that recovered some lines that (I assume) were removed accidentally but not recovered. Just check if the plots and terminal outputs look good.

Yes, that's perfect. I re-run the codes and it's good. Thanks a lot for your time and improvements.

YongchaoHuang commented 1 year ago

Looks good, I pushed a final commit that recovered some lines that (I assume) were removed accidentally but not recovered. Just check if the plots and terminal outputs look good.

I am adjusting the margin in plot(chain; margin=4.00mm) - making a final commit.

yebai commented 1 year ago

Excellent improvements @YongchaoHuang @devmotion!