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

New evaporation scheme #732

Merged
merged 1 commit into from
Sep 5, 2024
Merged

New evaporation scheme #732

merged 1 commit into from
Sep 5, 2024

Conversation

kmdeck
Copy link
Member

@kmdeck kmdeck commented Aug 21, 2024

Purpose

Large changes to src/standalone/Soil/energy_hydrology.jl, all else is updating scripts and tests

To-do

Content

Changes evaporation scheme by writing a new turbulent_fluxes method for EnergyHydrology.
Computes ice and liquid water fluxes separately, so ice_frac is no longer needed, and we now compute vapor_flux_ice
and vapor_flux_liq and store them separately.
Remove/comment out code from previous scheme (surface_resistance, dry_soil_thickness, tortuosity etc functions)
Update scripts/tests


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

@kmdeck kmdeck changed the title attempt New evaporation scheme Aug 22, 2024
experiments/long_runs/soil.jl Outdated Show resolved Hide resolved
@kmdeck kmdeck requested a review from AlexisRenchon August 27, 2024 00:09
@kmdeck kmdeck added the Run benchmarks Add this label to run benchmarks on clima label Aug 27, 2024
Copy link
Member

@AlexisRenchon AlexisRenchon 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 for the big PR! It looks great!

I bring two things up in this review:

  • do we need to add vapor_flux_ice to diagnostics in this PR?
  • going through this, I realized that it would be helpful to make a table in the docs containing as many acronyms as we can, with additional info ("stands for", "units", "comment"), somewhat akin to the diagnostics table, but with all sort of acronyms that we use throughout ClimaLand code. This table would probably be huge but very useful, and users would find what they look for via ctrl+f. Examples: Y, p, t, θ_i_sfc, z_0m, ts_in, etc. etc. We could refer to this table in many places.

(obviously this should be in another PR)

@@ -136,7 +136,7 @@ end
@diagnostic_compute "soil_aerodynamic_resistance" SoilCanopyModel p.soil.turbulent_fluxes.r_ae
@diagnostic_compute "soil_latent_heat_flux" SoilCanopyModel p.soil.turbulent_fluxes.lhf
@diagnostic_compute "soil_sensible_heat_flux" SoilCanopyModel p.soil.turbulent_fluxes.shf
@diagnostic_compute "vapor_flux" SoilCanopyModel p.soil.turbulent_fluxes.vapor_flux
@diagnostic_compute "vapor_flux" SoilCanopyModel p.soil.turbulent_fluxes.vapor_flux_liq # should add ice here
Copy link
Member

Choose a reason for hiding this comment

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

You mean add vapor_flux_ice?
But it currently doesn't exist?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, in this PR we added it. eventually it would be nice to have sublimation as a diagnostic also, and I could add it in this PR?

src/standalone/Soil/energy_hydrology.jl Outdated Show resolved Hide resolved
@AlexisRenchon AlexisRenchon mentioned this pull request Aug 30, 2024
36 tasks
@kmdeck kmdeck force-pushed the kd/evaporation_issues branch from d5352bb to 9e72938 Compare September 4, 2024 18:02
experiments/long_runs/land.jl Outdated Show resolved Hide resolved
@kmdeck kmdeck self-assigned this Sep 5, 2024
@kmdeck kmdeck force-pushed the kd/evaporation_issues branch from 2d318c4 to 8afd855 Compare September 5, 2024 16:30
@kmdeck
Copy link
Member Author

kmdeck commented Sep 5, 2024

For benchmarking - this PR:

[ Info: Timestep: 900.0 s
[ Info: Duration: 21600.0 s
[ Info: Starting profiling
[ Info: Num samples: 71
[ Info: Average time: 6.68 s
[ Info: Max time: 7.98 s
[ Info: Min time: 6.17 s
[ Info: Standard deviation time: 0.26 s

Previous main:

[ Info: Resolution: (101, 15)
[ Info: Timestep: 900.0 s
[ Info: Duration: 21600.0 s
[ Info: Starting profiling
[ Info: Num samples: 63
[ Info: Average time: 5.55 s
[ Info: Max time: 9.68 s
[ Info: Min time: 4.62 s
[ Info: Standard deviation time: 2.05 s
[ Info: Done profiling

I think this is not surprising given that we now call surface fluxes 2x per step for the soil model, rather than 1x.

@kmdeck kmdeck force-pushed the kd/evaporation_issues branch from a192884 to accec84 Compare September 5, 2024 18:02
@kmdeck kmdeck merged commit 1e8834c into main Sep 5, 2024
12 of 13 checks passed
@kmdeck kmdeck deleted the kd/evaporation_issues branch October 10, 2024 21:15
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 Run long runs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants