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

Replace Plots usage with Makie #1097

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

imreddyTeja
Copy link
Contributor

@imreddyTeja imreddyTeja commented Nov 22, 2024

All uses of Plots and ClimaCorePlots are removed in this commit. For almost all of the generated plots, the Makie version is visually similar to the Plots.jl versions.

Due to a limitation of Makie, the sea_breeze output animations are slightly different. The Plots.jl animations had a colorbar and levels that rescaled each frame. This commit makes the animations use a consistent colorbar and levels.

Plots and ClimaCorePlots are removed from Project.toml of both experiments/ClimaEarth and experiments/ClimaCore.

Some of the contour plots look slightly different due to differences in Makie and Plots.jl graphics, and because the default behavior of ClimaCoreMakie is to use viridis.

Purpose

Closes #1091

To-do

  • update manifests?
  • double check build kite artifacts vs an old buildkite-ci run

Content


  • I have read and checked the items on the review checklist.

@imreddyTeja imreddyTeja force-pushed the tr/remove-Plots.jl-usage branch 6 times, most recently from 73d2dd3 to 6cdf73f Compare November 25, 2024 17:16
@@ -83,7 +83,7 @@ plot_field_names(sim::Interfacer.SurfaceStub) = (:stub_field,)
)

output_plots = "test_debug"
@test_logs (:info, "plotting debug in test_debug") debug(cs, output_plots)
@test_logs (:info, "plotting debug in test_debug") match_mode = :any debug(cs, output_plots)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this because ClimaCoreMakie.fieldheatmap throws a warning because of an invalid attribute. I fixed the warning in ClimaCoreMakie, so this could be removed eventually

@imreddyTeja imreddyTeja force-pushed the tr/remove-Plots.jl-usage branch 5 times, most recently from b7beec5 to d86611e Compare November 26, 2024 16:03
All uses of Plots and ClimaCorePlots are removed in this commit.
For almost all of the generated plots, the Makie version is
visually similar to the Plots.jl versions.

Due to a limitation of Makie, the sea_breeze output animations
are slightly different. The Plots.jl animations had a colorbar and levels
that rescaled each frame. This commit makes the animations use a
consistent colorbar and levels.

Plots and ClimaCorePlots are removed from Project.toml of both
experiments/ClimaEarth and experiments/ClimaCore.

Some of the contour plots look slightly different due to
differences in Makie and Plots.jl graphics, and because the default
behavior of ClimaCoreMakie is to use viridis.
Using ClimaCoreMakie.fieldheatmap or fieldheatmap!
throws a warning because of an invalid attribute.
I fixed this in ClimaCoreMakie, but until the next
release, this allows other log messages in the test.
@imreddyTeja imreddyTeja marked this pull request as ready for review November 26, 2024 22:27
Copy link
Member

@juliasloan25 juliasloan25 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! Thank you Teja

@@ -8,6 +8,7 @@ ClimaAnalysis = "29b5916a-a76c-4e73-9657-3c8fd22e65e6"
ClimaAtmos = "b2c96348-7fb7-4fe0-8da9-78d88439e717"
ClimaComms = "3a4d1b5c-c61d-41fd-a00a-5873ba7a1b0d"
ClimaCore = "d414da3d-4745-48bb-8d80-42e94e092884"
ClimaCoreMakie = "908f55d8-4145-4867-9c14-5dad1a479e4d"
ClimaCorePlots = "cf7c7e5a-b407-4c48-9047-11a94a308626"
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove ClimaCorePlots and Plots from the test environment now too?

Copy link
Member

Choose a reason for hiding this comment

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

I see that we have some commented-out plotting lines in regridder_tests.jl. It's fine to remove those lines since they're not being used and we want to switch to ClimaUtilities regridding soon anyway. That might be the only place we use Plots/ClimaCorePlots in test/

import Plots
Plots.GRBackend()
import Makie
import CairoMakie

ARTIFACTS_DIR = joinpath("experiments/ClimaCore/output/heat-diffusion_artifacts")
Copy link
Member

Choose a reason for hiding this comment

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

There are no artifacts on buildkite for the heat diffusion run because this should be "/heat-diffusion/artifacts", but after rebasing on main it should be fine

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.

Remove Plots.jl dependency
2 participants