Skip to content
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

[WIP] Add documentation for Cloudy example #28

Merged
merged 8 commits into from
Jun 7, 2021
Merged

Conversation

bielim
Copy link
Contributor

@bielim bielim commented May 18, 2021

This PR adds documentation for the Cloudy example and fills a part of the documentation needs identified in #15.

  • Make Cloudy use the current master of Cloudy rather than the super-old V0.1.0
  • Make Cloudy_example_eki.jl consistent with terminology listed in the glossary
  • Make Cloudy_example_ukp.jl consistent with terminology listed in the glossary
  • Add documentation for Cloudy example
  • Review and finalize docs

@bielim bielim self-assigned this May 18, 2021
@bielim bielim added the documentation Improvements or additions to documentation label May 18, 2021
@bielim bielim requested review from odunbar and ilopezgp May 18, 2021 19:32
docs/src/examples/Cloudy_example.md Outdated Show resolved Hide resolved
docs/src/examples/Cloudy_example.md Outdated Show resolved Hide resolved
docs/src/examples/Cloudy_example.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@odunbar odunbar left a comment

Choose a reason for hiding this comment

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

My summary is maybe that the focus of this should be on Cloudy, and running the example. The inverse problem needs less detail here maybe - just define the prior, data and noise nice and clearly. Just because we do have a methods section and other docs for these!

Comments as I see them:

  1. Paragraph 2: suggestion: "Cloudy is initialized with a mass distribution of the cloud droplets; this distribution is then evolved in time by using a collision-coalescence kernel to represents the 2-particle interactions in a given time interval. The evolution is completed described by the shape of the initial distribution and the form of the kernel."
  2. Write out the formula here in the top section for the distribution and the kernel in this example
  3. How many moments are tracked - specify here. It is also not clear to me (yet) if cloudy is tracking only the moments, or if it evolves the distribution (of which you select N moments as your "data")
  4. "in realistic applications..." maybe you want to say: "... this parameter estimation procedure using Cloudy will be able to use a measurement of current droplet moments to obtain an estimated droplet mass distribution at a previous time".
  5. Prerequisites: Does cloudy have a release btw? Do we not have prereqs installed already on the EKP github (project.toml)?
  6. Inverse Problem: Is the Gamma distribution completely described in 3 moments?
  7. If we discuss the G map at the start, and the form of the inverse problem in methods sections of the docs , maybe focus here on priors for the parameters, Cloudy output, and the noise (e.g did you want to say what values of time T we use, and what values in the example do we take for the priors, and noise) also you can use some the code names too?
  8. We discuss the constrained-unconstrained space in detail in the method section and prior section - so can be more concise here. or can link other pages
  9. I like the solution/output/playing around sections. nice categories for users

PS Thanks a load for being the guinea pig with regards to how to structure and put information into these sections! You are helping everyone!

@@ -4,7 +4,7 @@ We provide the following template for how the tools may be applied.

For small examples typically have 2 files.

- `GModel.jl` Contains the forward map. The inputs should be the so-called free parameters we are interested in learning, and the output should be the measured data
- `DynamicalModel.jl` Contains the forward map. The inputs should be the so-called free parameters we are interested in learning, and the output should be the measured data
Copy link
Collaborator

@odunbar odunbar May 19, 2021

Choose a reason for hiding this comment

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

Note, the forward map is G = H \circ Psi \circ T - we could use the notation here to say DynamicalModel.jl contains H \circ Psi (can be viewed as either the forward map as applied to physical parameters, or the dynamical model \Psi with observed statistics H)

@bielim
Copy link
Contributor Author

bielim commented May 21, 2021

My summary is maybe that the focus of this should be on Cloudy, and running the example. The inverse problem needs less detail here maybe - just define the prior, data and noise nice and clearly. Just because we do have a methods section and other docs for these!

Comments as I see them:

  1. Paragraph 2: suggestion: "Cloudy is initialized with a mass distribution of the cloud droplets; this distribution is then evolved in time by using a collision-coalescence kernel to represents the 2-particle interactions in a given time interval. The evolution is completed described by the shape of the initial distribution and the form of the kernel."

I changed the original phrasing to:
Cloudy is initialized with a mass distribution of the cloud droplets; this distribution is then evolved in time, with more and more droplets colliding and combining into bigger drops according to the droplet-droplet interactions specified by a collision-coalescence kernel. The evolution is completely determined by the shape of the initial distribution and the form of the kernel.

  1. Write out the formula here in the top section for the distribution and the kernel in this example

I changed the structure of the document, such that there is now an "Overview" section followed by a "What Does Cloudy Do?" section. The formulas for the distribution and the kernel are written out in the latter section.

  1. How many moments are tracked - specify here. It is also not clear to me (yet) if cloudy is tracking only the moments, or if it evolves the distribution (of which you select N moments as your "data")

I tried to make this clearer by describing in some more detail how Cloudy works (section "What Does Cloudy Do?"): Cloudy only tracks three moments of the distribution which -- under the assumption that the distribution retains its Gamma shape -- is sufficient to determine the distribution parameters.

  1. "in realistic applications..." maybe you want to say: "... this parameter estimation procedure using Cloudy will be able to use a measurement of current droplet moments to obtain an estimated droplet mass distribution at a previous time".

I changed this to:
In more realistic applications, this parameter estimation procedure will use actual measurements of cloud properties to obtain an estimated droplet mass distribution at a previous time.

  1. Prerequisites: Does cloudy have a release btw? Do we not have prereqs installed already on the EKP github (project.toml)?

Yeah I don't know how to deal with this. It is true that the prerequisites are listed in Project.toml, but for this example I would like the user to install the current master branch of Cloudy (I don't know how to specify that in the .toml file). Cloudy does have a release (V0.1.0), but it is really old (that's why I wanted to use the master branch).

  1. Inverse Problem: Is the Gamma distribution completely described in 3 moments?

Yes, the Gamma moments and parameters are related such that you can uniquely determine the parameters from the first three moments of the distribution (for "regular" Gamma distributions you only need the first and second moment, but here we need three because we're working with scaled probability distributions, where the zeroth moment is not 1 but corresponds to the number of cloud droplets.

  1. If we discuss the G map at the start, and the form of the inverse problem in methods sections of the docs , maybe focus here on priors for the parameters, Cloudy output, and the noise (e.g did you want to say what values of time T we use, and what values in the example do we take for the priors, and noise) also you can use some the code names too?

The choice of priors and covariance as well as the Cloudy output are now described in more detail.

  1. We discuss the constrained-unconstrained space in detail in the method section and prior section - so can be more concise here. or can link other pages

I included a link, but didn't delete the part about constrained-unconstrained altogether, since we need it when talking about priors (and sometimes a bit of repetition doesn't hurt :))

  1. I like the solution/output/playing around sections. nice categories for users

PS Thanks a load for being the guinea pig with regards to how to structure and put information into these sections! You are helping everyone!

My pleasure :-D

@odunbar
Copy link
Collaborator

odunbar commented Jun 3, 2021

Looks great - very clear, lots of information for users, focused on the example.

What do you think about moving the Prerequisites and Structure to the very top of the page? You would need to move the sentence about the dynamical model into the end of "What does cloudy do?" section.

Otherwise, LGTM! Merge either way

@bielim
Copy link
Contributor Author

bielim commented Jun 7, 2021

Looks great - very clear, lots of information for users, focused on the example.

What do you think about moving the Prerequisites and Structure to the very top of the page? You would need to move the sentence about the dynamical model into the end of "What does cloudy do?" section.

Otherwise, LGTM! Merge either way

Yes, I was actually going back and forth between introducing Cloudy first vs. focusing on the code first, so the fact that you suggest putting the "code part" first definitely tips the scale in favor of this :-)
I've moved the sections on Prerequisites, Structure, as well as the section on Running the Example to the top of the document (right after the Overview), so that the information on how to get started is front and center, before we get into the more theoretical parts.

@bielim
Copy link
Contributor Author

bielim commented Jun 7, 2021

bors r+

@bors
Copy link
Contributor

bors bot commented Jun 7, 2021

Build succeeded:

@bors bors bot merged commit 05d4ac9 into main Jun 7, 2021
@bors bors bot deleted the mb/Cloudy_example_docs branch June 7, 2021 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants