-
Notifications
You must be signed in to change notification settings - Fork 19
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
Rewriting advancedvi docs #53
Rewriting advancedvi docs #53
Conversation
…riting_advancedvi_docs
…into rewriting_advancedvi_docs
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #53 +/- ##
==========================================
- Coverage 85.92% 81.81% -4.12%
==========================================
Files 11 11
Lines 199 209 +10
==========================================
Hits 171 171
- Misses 28 38 +10 ☔ View full report in Codecov by Sentry. |
Pull Request Test Coverage Report for Build 9328347532Warning: 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 |
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 @Red-Portal -- I added some minor comments below. Overall, it looks very good.
src/utils.jl
Outdated
@@ -32,3 +32,7 @@ function catsamples_and_acc( | |||
return (x, ∑y) | |||
end | |||
|
|||
function samples_expand_dim(x::AbstractVector) |
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 suggest we inline this function and add a comment, instead of creating an additional utility that can be used only once.
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 was thinking of using this as an abstraction for the future, where we get to use NamedTuple
-variate random variables. It would also make it easier to track where we explicitly assume which axes are what, which I think could be useful for maintenance.
Co-authored-by: Hong Ge <[email protected]>
Co-authored-by: Hong Ge <[email protected]>
Co-authored-by: Hong Ge <[email protected]>
…riting_advancedvi_docs
Co-authored-by: Hong Ge <[email protected]>
…ancedVI.jl into rewriting_advancedvi_docs
Co-authored-by: Hong Ge <[email protected]>
…ancedVI.jl into rewriting_advancedvi_docs
Co-authored-by: Hong Ge <[email protected]>
Co-authored-by: Hong Ge <[email protected]>
Co-authored-by: Hong Ge <[email protected]>
Co-authored-by: Hong Ge <[email protected]>
…ancedVI.jl into rewriting_advancedvi_docs
Co-authored-by: Hong Ge <[email protected]>
Co-authored-by: Hong Ge <[email protected]>
…ancedVI.jl into rewriting_advancedvi_docs
@yebai Thanks for your review! I tried to address most things, I think we are finally good to go? |
Thanks @Red-Portal! |
Documentations for the 0.3.0 rewrite.