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

Paper plots #565

Merged
merged 1 commit into from
May 11, 2024
Merged

Paper plots #565

merged 1 commit into from
May 11, 2024

Conversation

kmdeck
Copy link
Member

@kmdeck kmdeck commented Mar 27, 2024 •
edited
Loading

Purpose

  • Updates evaporation to be less dependent on resolution in the vertical (note: results look worse now)
  • ameliorates resolution dep by linearly interpolating to the surface (not efficient as written/allocates a lot)
  • Adds two tutorials which make plots included in the paper (moved one from experiments/)
  • moves get_deltaz to Domains, as the linear_interpolation_to_surface needs it

To-do

Content

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

@kmdeck kmdeck mentioned this pull request May 2, 2024
Open
8 tasks
@kmdeck kmdeck self-assigned this May 2, 2024
@kmdeck kmdeck added this to the O2.7: Land model papers milestone May 2, 2024
@kmdeck kmdeck marked this pull request as ready for review May 2, 2024 22:13
@kmdeck kmdeck requested a review from juliasloan25 May 3, 2024 00:03
@kmdeck kmdeck added documentation Improvements or additions to documentation enhancement New feature or request bugfix labels May 9, 2024
juliasloan25
juliasloan25 approved these changes May 10, 2024
View reviewed changes
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.

Looks good! I had a few comments, but I think it's good to merge

src/standalone/Soil/energy_hydrology.jl
S_l_sfc = p.soil.sfc_scratch
ClimaLand.Domains.linear_interpolation_to_surface!(S_l_sfc, S_l, cds.z)
θ_l_sfc = @. S_l_sfc * (ν - θ_r) + θ_r
θ_i_sfc = ClimaLand.Domains.top_center_to_surface(Y.soil.θ_i)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can change this to use linear_interpolation_to_surface now too, since we know that function isn't allocating a new field. Or we could have a separate PR to go through and replace top_center_to_surface with linear_interpolation_to_surface! throughout the code

Copy link
Member Author

Choose a reason for hiding this comment

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

I will change it here, and then open an issue for the other ones

Copy link
Member Author

Choose a reason for hiding this comment

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

oh wait. it will be a different issue. because we are in the same function, but we only have one allocated scratch spot to hold the values (instead of alllocating), we cant reuse it in the same function. I think we need multiple surface scratch fields...

Copy link
Member Author

Choose a reason for hiding this comment

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

noted here: #615

@kmdeck kmdeck merged commit f7c050b into main May 11, 2024
9 checks passed
@kmdeck kmdeck deleted the paper_plots branch May 11, 2024 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juliasloan25 juliasloan25 juliasloan25 approved these changes

Assignees

@kmdeck kmdeck

Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Milestone
O2.7: Land model papers
Development

Successfully merging this pull request may close these issues.
2 participants
@kmdeck @juliasloan25