-
Notifications
You must be signed in to change notification settings - Fork 11
Set default snow parameters to calibrated values #1137
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
This commit sets alpha_0, Delta_alpha, k, beta, and x0 to calibrated values in src/standalone/Snow/Snow.jl and experiments/long_runs/snowy_land.jl.
WalkthroughThis change updates the snow albedo parameterization in both the experiment setup and the core snow model code. In the experiment script, the parameters for the Possibly related PRs
Suggested labels
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
experiments/long_runs/snowy_land.jl
(2 hunks)src/standalone/Snow/Snow.jl
(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/standalone/Snow/Snow.jl (1)
Learnt from: kmdeck
PR: CliMA/ClimaLand.jl#1107
File: src/standalone/Snow/Snow.jl:109-116
Timestamp: 2025-04-17T15:19:46.548Z
Learning: The snow albedo model in ClimaLand.jl uses a density-dependent prefactor f(x) = min(1 - β(x-x0), 1) where x = ρ_snow/ρ_liq. This formula is correctly bounded at 1 by design through the min function.
🪛 GitHub Actions: JuliaFormatter
src/standalone/Snow/Snow.jl
[error] 294-303: Git diff detected formatting changes in Snow.jl around line 294. The indentation and alignment of parameters in the function SnowParameters were modified, indicating a formatting inconsistency.
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: ci 1.11 - windows-latest
- GitHub Check: ci 1.11 - macOS-latest
- GitHub Check: ci 1.11 - ubuntu-latest
- GitHub Check: ci 1.10 - windows-latest
- GitHub Check: downstream ClimaCoupler.jl (1.11)
- GitHub Check: test (1.11)
- GitHub Check: ci 1.10 - macOS-latest
- GitHub Check: docbuild
- GitHub Check: downstream ClimaCoupler.jl (1.10)
- GitHub Check: test (1.10)
- GitHub Check: ci 1.10 - ubuntu-latest
- GitHub Check: lib-climalandsimulations (1.11)
- GitHub Check: lib-climalandsimulations (1.10)
🔇 Additional comments (7)
src/standalone/Snow/Snow.jl (5)
86-91
: Minor docstring improvementsSmall readability improvement to the
AbstractAlbedoModel
docstring.
113-116
: Approved docstring clarificationSmall improvement to the explanation of the albedo formula parameters.
139-140
: Updated default parameters to calibrated valuesDefault values for β and x0 parameters have been updated to calibrated values, which should improve model accuracy.
- β changed from 0 to 0.7875
- x0 changed from 0.1046 to 0.2
This change makes the density-dependent prefactor active by default (previously β=0 disabled this effect).
151-154
: Improved docstring formattingSmall improvement to documentation structure.
161-175
: Improved docstring formattingMinor formatting changes to the documentation.
experiments/long_runs/snowy_land.jl (2)
172-172
: Minor whitespace fixRemoved trailing whitespace.
319-324
: Updated snow albedo parameters to match core modelUpdated the snow albedo model parameters to use the same calibrated values as defined in the core Snow.jl file:
- α_0: 0.64 → 0.7899
- Δα: 0.06 → 0.06575
- k: 2 → 17.92
- β: 0.4 → 0.7875
- x0: 0.2 → 0.1046
This ensures consistency between the experiment configuration and the core model defaults.
α_snow::AM = ZenithAngleAlbedoModel( | ||
FT(0.7899), # Calibrated α_0 | ||
FT(0.06575), # Calibrated Δα | ||
FT(17.92), # Calibrated k | ||
), # β and x0 default values are calibrated as well |
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.
Changed default snow albedo model to calibrated values
Major improvement: Changed the default snow albedo model from a simple constant value (ConstantAlbedoModel(0.8)
) to a more sophisticated zenith-angle-dependent model with calibrated parameters.
This change will provide more realistic snow albedo representation by default, accounting for both solar angle and snow density effects.
The pipeline reports a formatting issue around this line. Please fix the indentation to match the project's formatting guidelines:
#!/bin/bash
# Check the formatting error mentioned in the pipeline failure
grep -A 10 "Git diff detected formatting changes" pipeline_failures.txt
@@ -136,8 +136,8 @@ function ZenithAngleAlbedoModel( | |||
α_0::FT, | |||
Δα::FT, | |||
k::FT; | |||
β = FT(0), | |||
x0 = FT(0.2), | |||
β = FT(0.7875), # Calibrated value |
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 we can change the constructor to supply defaults for all parameters now, and then use this in the long run.
@@ -293,7 +293,11 @@ function SnowParameters{FT}( | |||
density::DM = MinimumDensityModel(FT(200)), | |||
z_0m = FT(0.0024), | |||
z_0b = FT(0.00024), | |||
α_snow::AM = ConstantAlbedoModel(FT(0.8)), |
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.
Some of our unit tests fail probably because they use this default.
Others fail probably because they are testing the default of the ZenithAngleAlbedo model and now that has changed.
can test locally with "julia --project=test test/standalone/Snow/test_file_name.jl" for each of the files in that directory.
Note that when we had the simpler model (only a dependence on zenith angle), we got: so it looks like most of variation in albedo is now being covered by the snow density portion of the parameterization, rather than the zenith angle portion. This is probably fine, but we may want to confirm at some point that the fit with all 5 parameters is improved enough, compared with that with only three parameters, so that the extra two parameters are justified. I mention this also because the zenith angle is completely known, so a parameterization based on that should be pretty stable, while snow density computed by the model could change as other things change in the model. @AlexisRenchon |
This commit sets alpha_0, Delta_alpha, k, beta,
and x0 to calibrated values in src/standalone/Snow/Snow.jl and experiments/long_runs/snowy_land.jl.