Deltares / Wflow.jl

Hydrological modeling
https://deltares.github.io/Wflow.jl/dev/
MIT License
116 stars 20 forks source link

Make unsatzone_flow_layer less convoluted #443

Open Huite opened 2 months ago

Huite commented 2 months ago

I was going through this function, and it took me very long to get it.

    sum_ast = 0.0
    # first transfer soil water > maximum soil water capacity layer (iteration is not required because of steady theta (usd))
    st = kv_z * min(pow(usd / l_sat, c), 1.0)
    st_sat = max(0.0, usd - l_sat)
    usd -= min(st, st_sat)
    sum_ast = sum_ast + min(st, st_sat)

sum_ast is initialized as zero, so it's equal to min(st, st_sat).

min(st, st_sat) has to be computed only once; might as well be bound directly to sum_ast.

Next is this bit:

    ast = max(min(st - min(st, st_sat), usd), 0.0)
    # number of iterations (to reduce "overshooting") based on fixed maximum change in soil water per iteration step (0.2 mm / model timestep)
    its = Int(cld(ast, 0.2))

ast is not the best name for max(min(st - min(st, st_sat), usd), 0.0), since it's only used to determine the number of iterations an not actually transferred. Hence my suggestion for remainder.

Additionally, max(min(st - min(st, st_sat), usd), 0.0) is unnecessarily complicated. min(st - sum_ast, usd) suffices, and is clearer.

In case you don't see it the equivalence immediately given the context (it took me long enough...):