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

Bring the docs up to speed #324

Merged
merged 1 commit into from
Feb 15, 2024
Merged

Bring the docs up to speed #324

merged 1 commit into from
Feb 15, 2024

Conversation

trontrytel
Copy link
Member

This PR brings the docs compile time back to reasonable. 112.712372 seconds (1.54 G allocations: 72.581 GiB, 5.14% gc time, 36.17% compilation time: 8% of which was recompilation) on my laptop. The reason for the slowdown was setting a very demanding tolerances for the ODE solver in our parcel model (especially if running with Float64 precision). I think by default we should run everything in Float32

Btw, when I was running individual simulations I noticed some of them were throwing warnings like:

WARNING: Multiple deposition nucleation parameterizations chosen to run in one simulation. Parcel will only run MohlerAF_Deposition for now.

Is that expected behavior @amylu00 ?

@trontrytel trontrytel added the documentation Improvements or additions to documentation label Feb 15, 2024
@trontrytel trontrytel requested a review from amylu00 February 15, 2024 05:04
@trontrytel trontrytel self-assigned this Feb 15, 2024
Copy link

codecov bot commented Feb 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (dfcdce7) 98.66% compared to head (999f005) 98.66%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #324   +/-   ##
=======================================
  Coverage   98.66%   98.66%           
=======================================
  Files          33       33           
  Lines        1047     1047           
=======================================
  Hits         1033     1033           
  Misses         14       14           

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

@trontrytel
Copy link
Member Author

It takes about 11 minutes to get all the dependencies and less than 4 minutes to generate the docs. We could make it shorter by for example deciding on using only one package for plotting. And by making the solver tolerances in parcel even less restrictive.

But maybe it's good enough. When building things locally it should be faster (about 2 minutes for me, not counting instantiating)

@amylu00
Copy link
Member

amylu00 commented Feb 15, 2024

Oh that might be wrong. I'll try fixing it sometime, it shouldn't be showing up for the examples we currently have. I think I mistakenly used or conditional statements when it should've been &&. Since there's so many modes being added, it'll have to be done another way anyways though. Thanks for the heads up!

Copy link
Member

@amylu00 amylu00 left a comment

Choose a reason for hiding this comment

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

Looks good to me

@trontrytel
Copy link
Member Author

Oh that might be wrong. I'll try fixing it sometime, it shouldn't be showing up for the examples we currently have. I think I mistakenly used or conditional statements when it should've been &&. Since there's so many modes being added, it'll have to be done another way anyways though. Thanks for the heads up!

Sounds good. I'll open an issue so that we don't forget

@trontrytel trontrytel merged commit 997ec7f into main Feb 15, 2024
10 checks passed
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.

2 participants