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

Surf/activation example with very different name #54

Merged
merged 1 commit into from
Oct 11, 2021

Conversation

trontrytel
Copy link
Member

@trontrytel trontrytel commented Sep 22, 2021

This PR adds an example to the documentation. The example shows how to calibrate the AerosolActivation module from CloudMicrophysics.jl package. The calibration is done in perfect model setting.

@odunbar
Copy link
Collaborator

odunbar commented Sep 22, 2021

I get an error after i instantiate:

    Status `~/Dropbox/Caltech/CESjl/EnsembleKalmanProcesses.jl/examples/AerosolActivation/Project.toml`
  [6eacf6c3] CLIMAParameters v0.2.0
  [6a9e3e04] CloudMicrophysics v0.3.0
  [31c24e10] Distributions v0.24.18
  [aa8a2aa5] EnsembleKalmanProcesses v0.1.0
  [91a5bcdd] Plots v1.22.1
  [37e2e46d] LinearAlgebra
  [44cfe95a] Pkg
  [9a3f8284] Random

and then call the example out of the box:

ERROR: LoadError: MethodError: no method matching total_N_activated(::EarthParameterSet, ::CloudMicrophysics.AerosolModel.AerosolDistribution{Tuple{CloudMicrophysics.AerosolModel.Mode_B{Tuple{Float64}, Float64}}}, ::Float64, ::Float64, ::Float64)
Closest candidates are:
  total_N_activated(::CLIMAParameters.AbstractParameterSet, ::CloudMicrophysics.AerosolModel.AbstractAerosolDistribution, ::FT, ::FT, ::FT, ::Thermodynamics.PhasePartition{FT}) where FT<:Real at /home/odunbar/.julia/packages/CloudMicrophysics/gL0Zn/src/AerosolActivation.jl:274
Stacktrace:
 [1] run_activation_model(molar_mass_calibrated::Float64, osmotic_coeff_calibrated::Float64)
   @ Main ~/Dropbox/Caltech/CESjl/EnsembleKalmanProcesses.jl/examples/AerosolActivation/aerosol_activation.jl:125
 [2] top-level scope
   @ ~/Dropbox/Caltech/CESjl/EnsembleKalmanProcesses.jl/examples/AerosolActivation/aerosol_activation.jl:139
in expression starting at /home/odunbar/Dropbox/Caltech/CESjl/EnsembleKalmanProcesses.jl/examples/AerosolActivation/aerosol_activation.jl:139

@trontrytel
Copy link
Member Author

I get an error after i instantiate:

    Status `~/Dropbox/Caltech/CESjl/EnsembleKalmanProcesses.jl/examples/AerosolActivation/Project.toml`
  [6eacf6c3] CLIMAParameters v0.2.0
  [6a9e3e04] CloudMicrophysics v0.3.0
  [31c24e10] Distributions v0.24.18
  [aa8a2aa5] EnsembleKalmanProcesses v0.1.0
  [91a5bcdd] Plots v1.22.1
  [37e2e46d] LinearAlgebra
  [44cfe95a] Pkg
  [9a3f8284] Random

and then call the example out of the box:

ERROR: LoadError: MethodError: no method matching total_N_activated(::EarthParameterSet, ::CloudMicrophysics.AerosolModel.AerosolDistribution{Tuple{CloudMicrophysics.AerosolModel.Mode_B{Tuple{Float64}, Float64}}}, ::Float64, ::Float64, ::Float64)
Closest candidates are:
  total_N_activated(::CLIMAParameters.AbstractParameterSet, ::CloudMicrophysics.AerosolModel.AbstractAerosolDistribution, ::FT, ::FT, ::FT, ::Thermodynamics.PhasePartition{FT}) where FT<:Real at /home/odunbar/.julia/packages/CloudMicrophysics/gL0Zn/src/AerosolActivation.jl:274
Stacktrace:
 [1] run_activation_model(molar_mass_calibrated::Float64, osmotic_coeff_calibrated::Float64)
   @ Main ~/Dropbox/Caltech/CESjl/EnsembleKalmanProcesses.jl/examples/AerosolActivation/aerosol_activation.jl:125
 [2] top-level scope
   @ ~/Dropbox/Caltech/CESjl/EnsembleKalmanProcesses.jl/examples/AerosolActivation/aerosol_activation.jl:139
in expression starting at /home/odunbar/Dropbox/Caltech/CESjl/EnsembleKalmanProcesses.jl/examples/AerosolActivation/aerosol_activation.jl:139

Right... I did release a new CloudMicrophysics.jl version this morning. Lemme fix the deps

@trontrytel
Copy link
Member Author

I updated the version of CloudMicrophysics.jl

@odunbar - do you know where to put the additional dependencies for the documenter build to be aware of them?

@odunbar
Copy link
Collaborator

odunbar commented Sep 29, 2021

@trontrytel have you tried adding them to the docs/Project.toml, and then add using statements in the docs/Make.jl? I believe this would be natural as it is the documenter that has to run the example file. It only doesnt need to add EnsembleKalmanProcesses.jl there because it instead locally adds this in Make.jl with the filepath

@navidcy wrote some nice literate docs for LossMinimization, and perhaps can concur?

@navidcy
Copy link
Contributor

navidcy commented Sep 29, 2021

Sure -- I can help out with that! :)
(after the Ocean Meeting Abstract deadline in a few hours :))

@navidcy
Copy link
Contributor

navidcy commented Oct 9, 2021

hey sorry, this slipped my radar! I'll get to it now!

@navidcy
Copy link
Contributor

navidcy commented Oct 9, 2021

A remark: when the docs are being build the use the docs/Project.toml file to load deps. If an example requires some deps outside of the EnsempleKalmanProcesses.jl package itself then they should be include in the docs/Project.toml. I added those deps (see 71fe662) and now docs build should be able to go through and produce a docs preview at:

https://clima.github.io/EnsembleKalmanProcesses.jl/previews/PR54/literated/aerosol_activation/

Copy link
Contributor

@navidcy navidcy left a comment

Choose a reason for hiding this comment

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

I made some comments. Seems like the example is still a work in progress?

examples/AerosolActivation/aerosol_activation.jl Outdated Show resolved Hide resolved
examples/AerosolActivation/aerosol_activation.jl Outdated Show resolved Hide resolved
examples/AerosolActivation/aerosol_activation.jl Outdated Show resolved Hide resolved
examples/AerosolActivation/aerosol_activation.jl Outdated Show resolved Hide resolved
Copy link
Contributor

@navidcy navidcy left a comment

Choose a reason for hiding this comment

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

Some more comments.

examples/AerosolActivation/aerosol_activation.jl Outdated Show resolved Hide resolved
examples/AerosolActivation/aerosol_activation.jl Outdated Show resolved Hide resolved
examples/AerosolActivation/aerosol_activation.jl Outdated Show resolved Hide resolved
examples/AerosolActivation/aerosol_activation.jl Outdated Show resolved Hide resolved
examples/AerosolActivation/aerosol_activation.jl Outdated Show resolved Hide resolved
examples/AerosolActivation/aerosol_activation.jl Outdated Show resolved Hide resolved
examples/AerosolActivation/aerosol_activation.jl Outdated Show resolved Hide resolved
examples/AerosolActivation/aerosol_activation.jl Outdated Show resolved Hide resolved
examples/AerosolActivation/aerosol_activation.jl Outdated Show resolved Hide resolved
@navidcy navidcy added the documentation Improvements or additions to documentation label Oct 9, 2021
@trontrytel
Copy link
Member Author

A remark: when the docs are being build the use the docs/Project.toml file to load deps. If an example requires some deps outside of the EnsempleKalmanProcesses.jl package itself then they should be include in the docs/Project.toml. I added those deps (see 71fe662) and now docs build should be able to go through and produce a docs preview at:

https://clima.github.io/EnsembleKalmanProcesses.jl/previews/PR54/literated/aerosol_activation/

Thank you for taking a look and fixing the dependencies! It's great to see the example building online. I'll go through your review comments right now and finish it.

@trontrytel
Copy link
Member Author

@navidcy - Thank you for all the comments and making it work! I applied the formatter and fixed a typo in plot name. So I hope that all four plots will show up now and that the formatter won't complain any more.

Can I squash/rebase and force push here to get rid of the cluttered history?

@navidcy
Copy link
Contributor

navidcy commented Oct 10, 2021

@navidcy - Thank you for all the comments and making it work! I applied the formatter and fixed a typo in plot name. So I hope that all four plots will show up now and that the formatter won't complain any more.

Can I squash/rebase and force push here to get rid of the cluttered history?

No worries!

Sure, do whatever you like! :)

@trontrytel trontrytel force-pushed the SURF/activation_example_with_very_different_name branch from b60687b to 7ff0aef Compare October 10, 2021 06:10
@navidcy
Copy link
Contributor

navidcy commented Oct 10, 2021

@trontrytel perhaps edit the first comment of the PR and possibly also its title to make it a bit more descriptive? It sort of reads a bit esoteric now.

@trontrytel
Copy link
Member Author

@trontrytel perhaps edit the first comment of the PR and possibly also its title to make it a bit more descriptive? It sort of reads a bit esoteric now.

Right... I just noticed that too :) No Jupyter notebook survived the process. Will update

@trontrytel trontrytel force-pushed the SURF/activation_example_with_very_different_name branch from 7ff0aef to a773fe1 Compare October 10, 2021 06:40
Copy link
Contributor

@navidcy navidcy left a comment

Choose a reason for hiding this comment

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

I added a few more suggestions regarding some typos, some simplifications to Julia code, adding markers in the ensemble mean plots, and rounding the estimated values.

Nice example!

examples/AerosolActivation/aerosol_activation.jl Outdated Show resolved Hide resolved
examples/AerosolActivation/aerosol_activation.jl Outdated Show resolved Hide resolved
examples/AerosolActivation/aerosol_activation.jl Outdated Show resolved Hide resolved
examples/AerosolActivation/aerosol_activation.jl Outdated Show resolved Hide resolved
examples/AerosolActivation/aerosol_activation.jl Outdated Show resolved Hide resolved
examples/AerosolActivation/aerosol_activation.jl Outdated Show resolved Hide resolved
examples/AerosolActivation/aerosol_activation.jl Outdated Show resolved Hide resolved
examples/AerosolActivation/aerosol_activation.jl Outdated Show resolved Hide resolved
@trontrytel trontrytel force-pushed the SURF/activation_example_with_very_different_name branch from fed2dc4 to 12d0cfe Compare October 10, 2021 15:49
@navidcy
Copy link
Contributor

navidcy commented Oct 10, 2021

@trontrytel how do you run the formatter offline? Apparently some of the changes I suggested were not inline with the formatter's style.

@trontrytel trontrytel force-pushed the SURF/activation_example_with_very_different_name branch from b82322c to 0842bd4 Compare October 10, 2021 17:18
@trontrytel
Copy link
Member Author

@trontrytel how do you run the formatter offline? Apparently some of the changes I suggested were not inline with the formatter's style.

fixed!

@navidcy
Copy link
Contributor

navidcy commented Oct 10, 2021

@trontrytel how do you run the formatter offline? Apparently some of the changes I suggested were not inline with the formatter's style.

fixed!

Nice!!

But still, if you know how to run the formatter offline can you tell me?

@trontrytel
Copy link
Member Author

@trontrytel how do you run the formatter offline? Apparently some of the changes I suggested were not inline with the formatter's style.

fixed!

Nice!!

But still, if you know how to run the formatter offline can you tell me?

Sure!

julia --project=.dev/ .dev/climaformat.jl file_to_be_formatted.jl

I'm superstitious so I run it on individual files that I know I changed. But you can look into the logs of the JuliaFormatter run and in the first command line of Apply JuliaFormatter he does the same but for all julia --project=.dev .dev/climaformat.jl .

@trontrytel
Copy link
Member Author

bors r+

bors bot added a commit that referenced this pull request Oct 11, 2021
54: Surf/activation example with very different name r=trontrytel a=trontrytel

This PR adds an example to the documentation. The example shows how to calibrate the [AerosolActivation module](https://github.com/CliMA/CloudMicrophysics.jl/blob/main/src/AerosolActivation.jl) from [CloudMicrophysics.jl](https://github.com/CliMA/CloudMicrophysics.jl) package. The calibration is done in perfect model setting. 

Co-authored-by: Anna Jaruga <[email protected]>
@trontrytel
Copy link
Member Author

bors r-

@bors
Copy link
Contributor

bors bot commented Oct 11, 2021

Canceled.

…ophysics.jl

Co-authored-by: montu12345 <[email protected]>
Co-authored-by: Navid C. Constantinou <[email protected]>
@trontrytel trontrytel force-pushed the SURF/activation_example_with_very_different_name branch from 0842bd4 to 36db5ae Compare October 11, 2021 15:43
@trontrytel
Copy link
Member Author

bors r+

@bors
Copy link
Contributor

bors bot commented Oct 11, 2021

Build succeeded:

@bors bors bot merged commit d330142 into main Oct 11, 2021
@bors bors bot deleted the SURF/activation_example_with_very_different_name branch October 11, 2021 15:54
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