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

Change albedo artifacts to use ClimaArtifacts #820

Merged
merged 2 commits into from
Oct 7, 2024

Conversation

imreddyTeja
Copy link
Contributor

@imreddyTeja imreddyTeja commented Oct 4, 2024

Both cesm2_albedo_dataset_path and bareground_albedo_dataset_path now use the new ClimaArtifact cesm2_albedo. This commit also deletes src/standalone/Bucket/artifacts, which was in an include, but never used.

Previously, the there were to functions in Artifacts.jl, bareground_albedo_dataset_path and cesm2_albedo_dataset_path. These now call a single function cesm2_albedo_dataset_folder and join path with the correct file name. To completely conform to the way other ClimaArtifacts are used, the two functions that return the specific file path should be removed. However, this would make the places the files are used messy, so I left the functions.

Note that esw_albedo_Amon_CESM2_historical_r1i1p1f1_gn_185001-201412_v2.nc is replaced with sw_albedo_Amon_CESM2_historical_r1i1p1f1_gn_185001-201412_v2_no-nans.nc, which is exactly the same.

For more details see related ClimaArtifacts PR here

Purpose

This PR progressed towards #580
It gets rid of ArtifactWrappers for land albedo and average bareground albedo.

To-do

Content


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

Both cesm2_albedo_dataset_path and bareground_albedo_dataset_path
now use the new ClimaArtifact cesm2_albedo. This commit also deletes
src/standalone/Bucket/artifacts, which was in an include, but never
used.
@imreddyTeja imreddyTeja added the Run benchmarks Add this label to run benchmarks on clima label Oct 4, 2024
@imreddyTeja imreddyTeja changed the title Change albedo artifascts to use ClimaArtifacts Change albedo artifacts to use ClimaArtifacts Oct 4, 2024
@imreddyTeja imreddyTeja marked this pull request as ready for review October 4, 2024 23:41
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.

This looks great! I think it's a good idea to keep the path functions - it's nice to abstract the specific filenames away from the user

@imreddyTeja imreddyTeja merged commit 6dc2192 into main Oct 7, 2024
9 of 11 checks passed
@imreddyTeja imreddyTeja deleted the tr/use-cesm2_albedo-artifact branch October 7, 2024 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Run benchmarks Add this label to run benchmarks on clima
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants