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

Store z etc in domain #658

Merged
merged 1 commit into from
Jun 18, 2024
Merged

Store z etc in domain #658

merged 1 commit into from
Jun 18, 2024

Conversation

kmdeck
Copy link
Member

@kmdeck kmdeck commented Jun 13, 2024 •
edited
Loading

Purpose

Every step of the soil model, we create the field of z and delta z_top, bottom. This stores them in the model domain object, along with z_sfc (which is used by some BC).

We no longer need to store dz_top in the phase change source term, so I removed that also.

To-do

Content

@kmdeck kmdeck self-assigned this Jun 13, 2024
@kmdeck kmdeck added this to the Maintenance and Improvements milestone Jun 13, 2024
@kmdeck kmdeck added enhancement New feature or request Run benchmarks Add this label to run benchmarks on clima labels Jun 13, 2024
@kmdeck kmdeck force-pushed the kd/store_z_in_cache branch 2 times, most recently from 47b8bc8 to 866836c Compare June 16, 2024 20:34
@kmdeck kmdeck requested a review from juliasloan25 June 16, 2024 21:06
@kmdeck kmdeck force-pushed the kd/store_z_in_cache branch from 866836c to e659127 Compare June 17, 2024 15:05
juliasloan25
juliasloan25 approved these changes Jun 17, 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.

This looks great, thank you!! I left some minor comments, but after those I think it's good to merge

src/shared_utilities/Domains.jl Outdated Show resolved Hide resolved
src/shared_utilities/Domains.jl Outdated Show resolved Hide resolved
src/shared_utilities/Domains.jl Outdated Show resolved Hide resolved
src/standalone/Soil/energy_hydrology.jl Outdated Show resolved Hide resolved
src/standalone/Soil/energy_hydrology.jl Outdated Show resolved Hide resolved
src/standalone/Soil/energy_hydrology.jl Show resolved Hide resolved
test/standalone/Soil/climate_drivers.jl Outdated Show resolved Hide resolved
@kmdeck kmdeck force-pushed the kd/store_z_in_cache branch from cdd7f1a to d74a775 Compare June 18, 2024 18:07
@kmdeck kmdeck enabled auto-merge June 18, 2024 18:07
@kmdeck kmdeck merged commit 7660e55 into main Jun 18, 2024
10 checks passed
@kmdeck kmdeck linked an issue Jul 2, 2024 that may be closed by this pull request
Reduce allocations and storage of z/Delta z #616
Closed
@kmdeck kmdeck mentioned this pull request Jul 2, 2024
Closed
@kmdeck kmdeck deleted the kd/store_z_in_cache branch October 10, 2024 21:14
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
enhancement New feature or request Run benchmarks Add this label to run benchmarks on clima
Projects
None yet
Milestone
Maintenance and Improvements
Development

Successfully merging this pull request may close these issues.

Reduce allocations and storage of z/Delta z
2 participants
@kmdeck @juliasloan25