Skip to content

Addition of additive coagulation example including comparison against PySDM and Droplets.jl #376

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

Merged
merged 43 commits into from
Jan 31, 2025

Conversation

emmacware
Copy link
Collaborator

No description provided.

@slayoo
Copy link
Member

slayoo commented Oct 24, 2024

  • include Julia Droplets.jl in the notebook
  • make the setup common also with the Julia code by using a dictionary of settings and writing it to a JSON file (as in the Trixi example in PyMPDATA)
  • switch settings and the plot construction to match Shima's box-model plot
  • add asserts wrt analytical solution
  • try if all works on Colab, mybinder and ARM JupyterHub
  • (sectional scheme via PartMC (Bott)) - for another PR
  • analytic (PartMC & PySDM)

@slayoo
Copy link
Member

slayoo commented Nov 26, 2024

@slayoo slayoo changed the title Addition of additive coagulation example including comparison against PySDM Addition of additive coagulation example including comparison against PySDM and Droplets.jl Dec 4, 2024
@slayoo
Copy link
Member

slayoo commented Dec 23, 2024

  • one thing we are certainly missing here is a reference to Shima's paper - perhaps we could add a markdown cell at the beginning explaining the setup, linking to the paper, and introducing the aim of comparing three models (with links to the model pages)

@slayoo
Copy link
Member

slayoo commented Dec 30, 2024

  • it is worth indicating if adaptivity is turned on or off for PySDM and Droplets.jl

@slayoo
Copy link
Member

slayoo commented Dec 31, 2024

@emmacware, trying to debug the Windows issue by logging into the CI machine, I ruled out any problems with Jupyter or all the execution machinery. The problem seems to be within Julia, is not deterministic, and the only symptom is that Julia quits silently at random stages of the computation returning exit code 127:

runneradmin@fv-az1212-152 MINGW64 /d/a/PyPartMC/PyPartMC
# julia.exe script.jl 
    Updating registry at `C:\Users\runneradmin\.julia\registries\General.toml`
   Resolving package versions...
  No Changes to `C:\Users\runneradmin\.julia\environments\v1.11\Project.toml`
  No Changes to `C:\Users\runneradmin\.julia\environments\v1.11\Manifest.toml`
Running simulation...
Time: 0.0 seconds
Time: 120.0 seconds
Time: 240.0 seconds
Time: 360.0 seconds
Time: 480.0 seconds

runneradmin@fv-az1212-152 MINGW64 /d/a/PyPartMC/PyPartMC
# echo $?                                                                                                                                                                                                          
127

runneradmin@fv-az1212-152 MINGW64 /d/a/PyPartMC/PyPartMC
# julia.exe script.jl 
    Updating registry at `C:\Users\runneradmin\.julia\registries\General.toml`
   Resolving package versions...
  No Changes to `C:\Users\runneradmin\.julia\environments\v1.11\Project.toml`
  No Changes to `C:\Users\runneradmin\.julia\environments\v1.11\Manifest.toml`
Running simulation...
Time: 0.0 seconds
Time: 120.0 seconds
Time: 240.0 seconds
Time: 360.0 seconds
Time: 480.0 seconds
Time: 600.0 seconds
Time: 720.0 seconds

runneradmin@fv-az1212-152 MINGW64 /d/a/PyPartMC/PyPartMC
# echo $?                                                                                                                                                                                                          
127

runneradmin@fv-az1212-152 MINGW64 /d/a/PyPartMC/PyPartMC
# julia.exe script.jl 
    Updating registry at `C:\Users\runneradmin\.julia\registries\General.toml`
   Resolving package versions...
  No Changes to `C:\Users\runneradmin\.julia\environments\v1.11\Project.toml`
  No Changes to `C:\Users\runneradmin\.julia\environments\v1.11\Manifest.toml`
Running simulation...
Time: 0.0 seconds

runneradmin@fv-az1212-152 MINGW64 /d/a/PyPartMC/PyPartMC
# echo $?
127

@slayoo slayoo requested a review from jcurtis2 January 10, 2025 22:17
Copy link
Member

@jcurtis2 jcurtis2 left a comment

Choose a reason for hiding this comment

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

In the spirit of the pyOpenSci feedback, I'm wondering if some more commenting/narrative could be applied. While it is clear to me what is going on, this notebook (I hope!) will be useful for either others to add their model for this coagulation case or follow this general framework for combining models to look at other things.

@emmacware emmacware requested a review from jcurtis2 January 24, 2025 15:32
@slayoo
Copy link
Member

slayoo commented Jan 28, 2025

thanks @jcurtis2 !
@emmacware, the last things I'd suggest to do are:

  • add README entry with badges
  • check if it works on colab (Julia!)
  • ditto fot ARM JupyterHub
  • ditto for mybinder
  • add JupyterHub badge
  • remove spurious Julia imports (e.g., DelimitedFiles seems not used)
  • indicate with a flag that adaptivity if off for Julia?

@codecov-commenter
Copy link

codecov-commenter commented Jan 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.70%. Comparing base (d472424) to head (b2a9ae0).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #376   +/-   ##
=======================================
  Coverage   89.70%   89.70%           
=======================================
  Files          44       44           
  Lines        2196     2196           
  Branches      111      111           
=======================================
  Hits         1970     1970           
  Misses        160      160           
  Partials       66       66           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@slayoo slayoo added this pull request to the merge queue Jan 31, 2025
Merged via the queue into open-atmos:main with commit 0908345 Jan 31, 2025
39 checks passed
@slayoo slayoo mentioned this pull request Feb 3, 2025
14 tasks
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