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 runoff method for single column #544

Closed
wants to merge 6 commits into from

Conversation

AlexisRenchon
Copy link
Member

@AlexisRenchon AlexisRenchon commented Mar 8, 2024

Purpose

This PR adds a method of runoff specifically for single site runs, which assumes that water at the top of the soil column (from precipitation or snow melt) which is in excess relative to infiltration capacity is runoff.

To-do

  • Method functions, BC conditions (! need docstring for abstractrunoffmodel)
  • Unit tests (! WIP)
  • Add runoff method in atmospheric drivers (? is this needed ?)
  • Call that method in experiments
  • Add docs

Content


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

@AlexisRenchon AlexisRenchon added the enhancement New feature or request label Mar 12, 2024
@AlexisRenchon
Copy link
Member Author

@juliasloan25
Hey Julia,
This PR should be pretty simple, we are just adding an AsbtractRunoffModel, but we get an error:

ERROR: The following 1 direct dependency failed to precompile:

ClimaLand [08f4d4ce-cf43-44bb-ad95-9d2d5f413532]

Failed to precompile ClimaLand [08f4d4ce-cf43-44bb-ad95-9d2d5f413532] to "/home/runner/.julia/compiled/v1.10/ClimaLand/jl_xbjQuI".
ERROR: LoadError: TypeError: in Type{...} expression, expected UnionAll, got Type{ClimaLand.Soil.Runoff.InfiltrationExcess}
Stacktrace:
  [1] top-level scope
    @ ~/work/ClimaLand.jl/ClimaLand.jl/src/standalone/Soil/boundary_conditions.jl:1001

and I am not sure why it happens... (very similar code just above in boundary_conditions.jl to define the TOPMODELRunoff methods do not have this error).
Note I am also surprised the error is at line 1001, which is the beginning of a docstring

@juliasloan25
Copy link
Member

@juliasloan25 Hey Julia, This PR should be pretty simple, we are just adding an AsbtractRunoffModel, but we get an error:

ERROR: The following 1 direct dependency failed to precompile:

ClimaLand [08f4d4ce-cf43-44bb-ad95-9d2d5f413532]

Failed to precompile ClimaLand [08f4d4ce-cf43-44bb-ad95-9d2d5f413532] to "/home/runner/.julia/compiled/v1.10/ClimaLand/jl_xbjQuI".
ERROR: LoadError: TypeError: in Type{...} expression, expected UnionAll, got Type{ClimaLand.Soil.Runoff.InfiltrationExcess}
Stacktrace:
  [1] top-level scope
    @ ~/work/ClimaLand.jl/ClimaLand.jl/src/standalone/Soil/boundary_conditions.jl:1001

and I am not sure why it happens... (very similar code just above in boundary_conditions.jl to define the TOPMODELRunoff methods do not have this error). Note I am also surprised the error is at line 1001, which is the beginning of a docstring

Hi Alexis! Usually when the error points to the start of a docstring, the error is actually in the line of code immediately after that docstring. In this case, the error is coming up because you dispatch on <: InfiltrationExcess{FT} (here), but InfiltrationExcess isn't defined to take a type parameter FT. Locally, I removed {FT} from the dispatch and it precompiles fine :)

@AlexisRenchon
Copy link
Member Author

@braghiere @kmdeck Do we still want to do this? Or should we close this PR?

@braghiere
Copy link
Member

Well, we need the runoff term for site level runs. Can we quickly finish it and merge it?

@kmdeck
Copy link
Member

kmdeck commented Jul 19, 2024

completed following this PR's approach in PR #702

@kmdeck kmdeck closed this Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants