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.

Sign up for GitHub

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

Jump to bottom

Update EDMF plots #2574

Merged
merged 1 commit into from
Feb 4, 2024
Merged

Update EDMF plots #2574

merged 1 commit into from
Feb 4, 2024

Conversation

nefrathenrici
Copy link
Member

@nefrathenrici nefrathenrici commented Jan 25, 2024 •
edited
Loading

Refactors make_plots_generic and customizes the EDMF line plots for readability.

Content

make_plots_generic

  • creates a grid of plots (default only 1 column) and can take in a custom plotting function.
  • takes a summary_files kwarg to prepend initial pages to the final PDF.

EDMF plots

  • plot gridmean/updraft on the same axis for vertical profiles
  • make plots on a 4x2 grid
  • Remove y log scale
  • Add more variables

@nefrathenrici nefrathenrici linked an issue Jan 25, 2024 that may be closed by this pull request
Improve EDMF plots #2539
Closed
Sbozzolo
Sbozzolo reviewed Jan 25, 2024
View reviewed changes
post_processing/ci_plots.jl Outdated Show resolved Hide resolved
post_processing/ci_plots.jl Outdated Show resolved Hide resolved
szy21
szy21 reviewed Jan 26, 2024
View reviewed changes
Copy link
Member

@szy21 szy21 left a comment

Choose a reason for hiding this comment

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

The plots look great, thanks! Just notice that it is still one contour plot per row - is this intended?

post_processing/ci_plots.jl Show resolved Hide resolved
post_processing/ci_plots.jl Outdated Show resolved Hide resolved
@nefrathenrici
Copy link
Member Author

The plots look great, thanks! Just notice that it is still one contour plot per row - is this intended?

@szy21 There seem to be issues with gridding the contour plots. I believe it has to do with the color bar, but need to look into it properly.
Screenshot 2024-01-26 at 10 27 40 AM

@szy21
Copy link
Member

szy21 commented Jan 26, 2024

Ah, ok, then what you have now is good. Thanks!

@nefrathenrici nefrathenrici force-pushed the ne/edmf_plots branch 4 times, most recently from 4dbff90 to 1fb9a74 Compare January 26, 2024 21:08
@nefrathenrici nefrathenrici force-pushed the ne/edmf_plots branch 3 times, most recently from 819ec7f to 062451e Compare January 30, 2024 23:27
Sbozzolo
Sbozzolo reviewed Jan 31, 2024
View reviewed changes
post_processing/ci_plots.jl Outdated Show resolved Hide resolved
post_processing/ci_plots.jl Outdated Show resolved Hide resolved
post_processing/ci_plots.jl Outdated Show resolved Hide resolved
post_processing/ci_plots.jl Outdated Show resolved Hide resolved
Sbozzolo
Sbozzolo reviewed Feb 2, 2024
View reviewed changes
post_processing/ci_plots.jl Outdated Show resolved Hide resolved
post_processing/ci_plots.jl Outdated Show resolved Hide resolved
post_processing/ci_plots.jl Outdated Show resolved Hide resolved
Sbozzolo
Sbozzolo reviewed Feb 2, 2024
View reviewed changes
Copy link
Member

@Sbozzolo Sbozzolo left a comment

Choose a reason for hiding this comment

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

This looks great. Only cosmetic comments are left.

Sbozzolo
Sbozzolo approved these changes Feb 4, 2024
View reviewed changes
Copy link
Member

@Sbozzolo Sbozzolo left a comment

Choose a reason for hiding this comment

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

Thank you! This looks really nice and clean. I am very happy on how it turned out!

Just squash your commits and feel free to merge.

@nefrathenrici nefrathenrici added this pull request to the merge queue Feb 4, 2024
Merged via the queue into main with commit c7745a5 Feb 4, 2024
9 of 10 checks passed
@nefrathenrici nefrathenrici deleted the ne/edmf_plots branch February 4, 2024 22:58
@szy21
Copy link
Member

szy21 commented Feb 5, 2024

Thank you both!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@szy21 szy21 szy21 left review comments

@Sbozzolo Sbozzolo Sbozzolo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve EDMF plots
3 participants
@nefrathenrici @szy21 @Sbozzolo