Skip to content

Conversation

@masenka31
Copy link
Member

Adding a new generative model working on sets of data. Work in progress now.

@masenka31 masenka31 marked this pull request as draft February 8, 2021 08:50
Copy link
Member

@nmheim nmheim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff, thanks a lot for the PR! I requested some minor changes in the code :)

Copy link
Member

@nmheim nmheim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, we are almost there now! In order to create a new release you just need to increase the version number of GenerativeModels in the Project.toml (from 0.2.2 to 0.2.3). then the julia tagbot will automatically create a new release an register it in the julia registry, so that we can install/update the package via ]add Generativemodels/]update :)

function as
`loss(x) = lossNS(x, NS)`
`loss(x) = -elbo(NeuralStatistician,x)`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be misunderstood. It looks like the elbo would only get a NeuralStatistician type instead of an actual object. maybe something like this:

model = NeuralStatistician(cdim, instance_enc, enc_c_dist, cond_z_dist, enc_z_dist, dec_dist)
loss(x) = - elbo(model,x)

In that case it would also be really great to have short explanations of the variables in there like instance_enc. but if you are pressed with time we can just merge it with the corrections above.

@codecov-io
Copy link

Codecov Report

Merging #96 (3e63c15) into master (f463742) will decrease coverage by 7.24%.
The diff coverage is 53.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #96      +/-   ##
==========================================
- Coverage   81.81%   74.57%   -7.25%     
==========================================
  Files           5        6       +1     
  Lines          88      118      +30     
==========================================
+ Hits           72       88      +16     
- Misses         16       30      +14     
Impacted Files Coverage Δ
src/GenerativeModels.jl 100.00% <ø> (ø)
src/statistician.jl 53.33% <53.33%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f463742...202c35e. Read the comment docs.

@masenka31
Copy link
Member Author

I updated the documentation for NeuralStatistician to include an example and rewrote the docs for elbo to be more accurate. I think there should be no more problems.

@nmheim nmheim marked this pull request as ready for review February 10, 2021 18:52
@nmheim
Copy link
Member

nmheim commented Feb 10, 2021

Looks great, thank you! :)

@nmheim nmheim changed the title Statistician Implement NeuralStatistician Feb 10, 2021
@nmheim nmheim merged commit 9c2833b into master Feb 10, 2021
@nmheim nmheim deleted the statistician branch February 10, 2021 18:54
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.

4 participants