-
Notifications
You must be signed in to change notification settings - Fork 14
Spatially varying canopy height #1502
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
base: main
Are you sure you want to change the base?
Conversation
Previously h_leaf was hardcoded to FT(1), now it uses toml_dict["canopy_height"] to ensure consistency between biomass and hydraulics height parameters.
…acts - Reads spatially varying canopy height from CLM vegetation_properties_map.nc - Returns ClimaCore Field with height values for each grid cell - Follows same pattern as clm_rooting_depth() and clm_medlyn_g1() - Heights are based on dominant PFT from CLM5 MONTHLY_HEIGHT_TOP data
CLM data uses 'z_top' (Canopy top height) not 'canopy_height'. Data investigation findings: - 192x288 grid (0.9x1.25 degrees) - Height range: 0-40.13m - 92.5% < 1m (grasslands/crops) - 7.4% >= 10m (4068 cells, tall forests) - Max height: 40.13m (tropical forests) - Median: 0m (sparse/no vegetation dominant)
- Add HTH type parameter to support Union{FT, Field} for height
- Update struct definition in biomass.jl to accept both scalar and Field height
- Update outer constructor to pass height type to struct instantiation
- Update Canopy.jl constructor to include height type parameter
- This allows height to be either a scalar (backward compatible) or a Field (new functionality)
- Implement effective_canopy_height() to cap heights below atmospheric reference - Default buffer of 2m ensures adequate space for flux calculations - Logs warning with statistics when capping occurs - Addresses constraint that ERA5 forcing is at 10m reference height - Prevents numerical issues from canopy extending above atmospheric layer
- Import clm_canopy_height and effective_canopy_height functions - Read canopy height from CLM artifact data in snowy_land_pmodel.jl - Apply capping to 8m (10m atmospheric height - 2m buffer) - Pass spatially-varying height to PrescribedBiomassModel - This replaces the previous fixed 1m canopy height with realistic CLM values
Tests added: 1. spatial_parameters.jl: - Test clm_canopy_height() reads CLM data correctly - Test effective_canopy_height() capping with default and custom buffers - Verify proper field structure and value constraints 2. test_spatially_varying_canopy_height.jl (new file): - Test PrescribedBiomassModel with scalar height (backward compatibility) - Test PrescribedBiomassModel with Field height (new functionality) - Test integration with CLM height data and capping - Test full CanopyModel construction with spatially-varying height All tests verify: - Correct Field axes and types - Height constraints (capping below atmospheric reference) - Non-negative heights - Backward compatibility with scalar heights
Create experiment script to compare: - Run 1: Default configuration with height=1m everywhere - Run 2: Spatially-varying height from CLM data (capped at 8m) Both runs use same 1-year period (2008-03-01 to 2009-03-01) to enable direct comparison of model outputs and assess impact of spatially-varying canopy height on simulated fluxes and states.
- Replace count() with ifelse() to avoid Boolean Fields on GPU - Use float mask (1.0/0.0) instead of boolean for counting - Fixes 'Bool cannot be represented using Float64' error on CUDA
- Remove Int() conversion that caused overflow with large GPU arrays - Keep n_capped_sum as Float for display - Use round() for cleaner output
…t_height() - Changed FT(0.67) * height to FT(0.67) .* height - Fixes MethodError when height is a Field (spatially-varying) - Maintains backward compatibility with scalar height
- Changed biomass.height == hydraulics_height to use all(isapprox.(...)) - Original == with Field was creating Field of booleans (very slow) - Maintains backward compatibility with scalar height using ≈ - This was causing 3x+ slowdown in spatially-varying height runs
These simulation outputs have been moved to ~/climaland_outputs/ for local analysis and are not part of the core feature implementation.
Wrap parent() array accesses with Array() to convert from GPU to CPU arrays before scalar indexing. This prevents 'Scalar indexing is disallowed' errors when running tests on GPU.
| isapprox.(biomass.height, hydraulics_height, rtol = 1e-10), | ||
| ) "Biomass height Field must match hydraulics height" | ||
| else | ||
| @assert biomass.height ≈ hydraulics_height "Biomass height must match hydraulics height" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont quite understand how this works, because the hydraulics_height is a scalar. How can this pass when biomass.height is spatialy varying?
In general we need to go through src and make sure these are consistent, because in some places we use the 'hydraulics" height as the height
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to document what biomass height means and what hydraulics height means, and what they're each used for
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right - the hydraulics compartments are always scalar (1m). When biomass.height varies spatially, only the aerodynamic calculations use those varying heights. The hydraulics structure stays uniform everywhere.
This is a current limitation - we can extend hydraulics to support spatial variation in a follow-up PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check still doesn't make sense - if biomass.height has different values throughout the domain, it doesn't make sense to assert that it's approximately equal to hydraulics_height everywhere (in the if branch lines 720-723)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with Julia's last comment. I dont quite see how it passes right now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part:
@assert all(
isapprox.(biomass.height, hydraulics_height, rtol = 1e-10),
) "Biomass height Field must match hydraulics height everywhere"
kmdeck
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's try running this and see how the SHF, LHF, etc change compared to observations
Moving forwards, I think there are two ways this can be extended:
- make a canopy roughness model type. right now we are using physical quantities (height) to compute roughness and displacement height (using physical equations someone has come up with). Something more flexible would be to predict roughness, displacement height, from inputs like LAI, canopy height, etc using a data driven model. Having a type for "canopy roughness" would let us switch between the two easily. (I am separately adding this already in another PR)
- in Biomass - create a type of "height model": this would be prescribed (constant or spatially varying) for now, PrescribedCanopyHeight. Then in the future we can also easily make different parameterizations for canopy height (e.g. prognostic, predicting it from input/time varying data). but this seems very far in the future compared to the first point
| isapprox.(biomass.height, hydraulics_height, rtol = 1e-10), | ||
| ) "Biomass height Field must match hydraulics height" | ||
| else | ||
| @assert biomass.height ≈ hydraulics_height "Biomass height must match hydraulics height" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to document what biomass height means and what hydraulics height means, and what they're each used for
| canopy_height = SpaceVaryingInput( | ||
| joinpath(clm_artifact_path, "vegetation_properties_map.nc"), | ||
| "z_top", | ||
| surface_space; | ||
| regridder_type, | ||
| regridder_kwargs = (; extrapolation_bc, interpolation_method), | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could provide the effective_canopy_height as a preprocessing function instead, so we don't have to think about it wherever we access the canopy height data. This would be safer because we'll avoid users forgetting to apply the clipping.
file_reader_kwargs = (; preprocess_func = (data) -> effective_canopy_height(data, z_atm; buffer),),
And then we take z_atm and buffer as inputs to this function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
effective_canopy_height would also have to change to take in a float canopy_height instead of the whole field, since here it'll be broadcasted over the field. We wouldn't be able to log the information you have in the warning, but I think that's okay. Usually when we add warnings like this we regret it later because it clutters the simulation output
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think this would be a good change - then we won't have to call effective_canopy_height every time we read in canopy height from a map
…ng, clarify docs, update tests, and clean up experiment scripts
Largest changes are observed: Sensible Heat (ERA5): Bias ↓ (8.27 → 7.89 W m⁻²) ~5% improvement, RMSE same. Evaporative Fraction (ERA5): Bias slightly closer to 0 (−0.0626 → −0.0601) ~4% improvement, Phase same. (No RMSE.) Surface Upward LW (CERES): Bias closer to 0 (−5.85 → −5.52 W m⁻²) ~6% improvement, RMSE ~ +0.1, Phase ≈ same. All the other variables have minimal changes. |
- Replace deprecated canopy_height() with clm_canopy_height() - Rename variable to raw_height for clarity
| @assert typeof(photosynthesis) <: PModel{FT} "When using PModelConductance for stomatal conductance, you must also use PModel for photosynthesis" | ||
| end | ||
|
|
||
| # Height consistency check between biomass and hydraulics: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you move this comment into the docstring? That way it'll be visible in the docs too
| # hydraulics_height: | ||
| # - Used for: Vertical structure of plant water transport (defines compartment spacing) | ||
| # - Type: Currently always a scalar | ||
| # - Computed from: hydraulics.compartment_surfaces[end] - hydraulics.compartment_surfaces[1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't say that the hydraulics height defines the compartment spacing since it's computed from the compartment spacing here. I also don't see it used outside of this comparison to biomass height, unless I'm missing something
| # Height consistency check between biomass and hydraulics: | ||
| # | ||
| # biomass.height: | ||
| # - Used for: Aerodynamic calculations (displacement height, roughness length) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that the biomass.height is used in displacement_height, but it looks like the roughness lengths still independently come from the toml dict here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think this PR just adjusts displacement height. The roughness lengths are either constant (set in the toml, the default) or computed like so:
ClimaLand.jl/experiments/long_runs/snowy_land_pmodel.jl
Lines 97 to 99 in 7787892
| h_canopy = hydraulics.compartment_surfaces[end] | |
| z_0m = FT(0.13) * h_canopy | |
| z_0b = FT(0.1) * z_0m |
but they are required to be scalar.
I am adding a PR which allows these to be spatially varying, among other things.
| This typically affects ~7% of global cells, primarily in regions with tall forests | ||
| (e.g., tropical rainforests, temperate forests in Patagonia and New Zealand). | ||
| """ | ||
| function effective_canopy_height( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this OK for now! In the future, we can adjust it. because, what actually matters (at least in standard Monin-Obukhov, I think) is not quite z_atm - h_canopy, but z_atmos - displacement height. so I think this will transform to be
d = displacement_height(canopy_height..., LAI, maxLAI,....)
effective_d = min(d, z_atm - buffer)
and perhaps we even compute this when we compute the surface fluxes, not as an a-priori step (esp if displacement height changes in time)
| using ClimaLand.Snow | ||
| using ClimaLand.Soil | ||
| using ClimaLand.Canopy | ||
| using ClimaLand.Canopy: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we can revert this file for now (the PR adds in the spatially varying height capability) but does not use yet in global runs?
kmdeck
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good, but I dont love that we now have multiple definitions of height, and have some pause since we arent sure we want to predict canopy height in the future.
I wonder, since these additions affect the turbulent boundary fluxes only, and because the height only affects the roughness lengths and displacment height but is not used directly itself, if we should move height out of the biomass model and only compute displacement and roughness length fields using the height once, and store those, and not the height.
This isnt really possible in this PR, but after PR #1505 merges, I think we can do something like this.
Purpose
Add support for spatially-varying canopy height in ClimaLand vegetation model, replacing the fixed 1m default with realistic height data from CLM artifacts (capped at 8m).
Content
Core Implementation
clm_canopy_height()function to read spatially-varying canopy height from CLM artifact data (src/standalone/Vegetation/spatially_varying_parameters.jl)effective_canopy_height()function to cap heights at 8m for numerical stability while preserving spatial patternsPrescribedBiomassModelto support spatially-varying height through type parameterHTH(canopy height type)src/standalone/Vegetation/canopy_boundary_fluxes.jl)h_leafsynchronization withcanopy_heightinPlantHydraulicseffective_canopy_heightBoolean operationsTesting and Validation
Impact Assessment
Figure: Sensible heat flux comparison showing Run 1 (default 1m height), Run 2 (spatially-varying height), absolute difference, and percent change.
The spatially-varying canopy height primarily affects:
To-do