Precipitation rate <= 0 #6
Conversation
… a basic example with common usage of orographic_precipitation within Fastscape and minor edits to the documentation.
There was a problem hiding this comment.
Thanks @EstebanAce for this PR! (Sorry I completely missed it).
I left a few minor comments.
| @@ -28,6 +28,7 @@ def compute_orographic_precip(elevation, dx, dy, **param): | |||
| nm (float) : moist stability frequency [s-1] | |||
| hw (float) : water vapor scale height [m] | |||
| cw (float) : uplift sensitivity [kg m-3], product of saturation water vapor sensitivity ref_density [kg m-3] and environmental lapse rate (lapse_rate_m / lapse_rate) | |||
| min_precip (float) : minimum precipitation | |||
There was a problem hiding this comment.
nit: let's rename it to precip_min to be consistent with precip_base. Could you also move it next to precip_base in the docstrings here and add a bit more description (cf. code comment below)
| @@ -63,39 +68,44 @@ class OrographicPrecipitation: | |||
| cw = xs.variable(description="uplift sensitivity", intent="out", | |||
| attrs={"units": "kg/m^3"}) | |||
|
|
|||
| min_precip = xs.variable(description="minimum precipitation", | |||
There was a problem hiding this comment.
Same comment here: min_precip -> precip_min + move next to precip_base.
| @@ -43,7 +47,8 @@ class OrographicPrecipitation: | |||
| precip_base = xs.variable(description="background, non-orographic precipitation rate", | |||
| attrs={"units": "mm/h"}) | |||
| rainfall_frequency = xs.variable(description="daily rainfall frequency", | |||
| attrs={"units": "1/day"}, intent='inout') | |||
| default=1, | |||
| attrs={"units": "1/day"}, intent='inout') | |||
There was a problem hiding this comment.
Any reason why rainfall_frequency is "inout"? This should be "out" I think. Also, it should probably be moved out of this process since it's not used here (see #4 (comment)), but we can do this in a follow-up PR.
There was a problem hiding this comment.
No, I don't have a particular reason why it should be "inout", but yeah let's look into it in another PR.
|
Thanks for your suggestions Benoît, I Implemented them and sure let's look into the "inout" and the spatially variable in another PR. Hannah did some work on that and I think soon we can refine that part. |
There was a problem hiding this comment.
Thanks @EstebanAce for the updates!
Just one minor comment below about the name chosen for the xsimlab process.
|
Looks all good, thanks @EstebanAce! Merging. |
Added a fix I worked with Jean to prevent precipitation rate from going below or equal to zero, preventing problems while being computed with fastcape.